Hello Arne
I created a small test case with your exact code (wrapped in a mock processor class to run in a test case) using googletest and which as you pointed out is only doing memory management on the ui thread. Here is the test case:
struct MyParams {
double v1;
bool v2;
};
class Processor
{
public:
std::atomic<MyParams *> setStateParams{nullptr};
std::atomic<MyParams *> toDeleteParams{nullptr};
MyParams processParams{0, true};
~Processor()
{
cleanupMemory();
}
void process()
{
auto stateParams = setStateParams.exchange(nullptr);
if(stateParams)
{
processParams = *stateParams;
auto test = toDeleteParams.exchange(stateParams);
DCHECK_F(test == nullptr);
}
}
void cleanupMemory()
{
auto toDelete = toDeleteParams.exchange(nullptr); // cleanup memory
if(toDelete)
delete toDelete;
toDelete = setStateParams.exchange(nullptr); // cleanup memory
if(toDelete)
delete toDelete;
}
void setState(MyParams const ¶ms)
{
cleanupMemory();
// allocate new state
auto stateParams = new MyParams{params.v1, params.v2};
// read state from stream into stateParams !
setStateParams.exchange(stateParams);
}
};
TEST(Processor, MultiThreadSafe)
{
constexpr int N = 10000;
Processor processor;
auto ui = [&] {
for(int i = 0; i < N; i++)
{
processor.setState(MyParams{static_cast<double>(i), true});
}
};
auto processing = [&] {
for(int i = 0; i < N; i++)
{
processor.process();
}
};
std::thread uiThread(ui);
std::thread processingThread(processing);
uiThread.join();
processingThread.join();
}
After running a few iterations, the test fails
Running main() from gtest_main.cc
Stack trace:
5 0x7fff59172c5d thread_start + 13
4 0x7fff5917356d _pthread_body + 0
3 0x7fff591736c1 _pthread_body + 340
2 0x10cdf48e5 void* std::__thread_proxy<std::tuple<std::unique_ptr<std::__thread_struct, std::default_delete<std::__thread_struct> >, pongasoft::VST::Test::Processor_MultiThreadSafe_Test::TestBody()::$_5> >(void*) + 5171 0x10cdf4b64 pongasoft::VST::Test::Processor_MultiThreadSafe_Test::TestBody()::$_5::operator()() const + 520 0x10cdf4e25 pongasoft::VST::Test::Processor::process() + 677
2018-06-18 14:17:04.040 ( 0.001s) [ A41B] test-concurrent.cpp:318 FATL| CHECK FAILED: test == nullptr
Process finished with exit code 134 (interrupted by signal 6: SIGABRT)
This demonstrates that the code is actually not thread safe even if there is only 1 ui thread calling setState and 1 processing thread calling process…
I know you said “typos may very well happened” but since I have been struggling so much with this, I have been trying to create my own version without any success whatsoever. Do you know have a version of this code that is thread safe that you could point me too?
This seems to be an incredibly complicated problem and I really think that the VST SDK should provide at a minimum a solution for implementing both getState and setState (which have different requirements) in a thread safe, lock free, memory-allocation-on-the-ui-thread-only fashion…
Thanks
Yan