COCKOS
CONFEDERATED FORUMS
Cockos : REAPER : NINJAM : Forums
Forum Home : Register : FAQ : Members List : Search :

Go Back   Cockos Incorporated Forums > Other Software Discussion > WDL users forum

Reply
 
Thread Tools Display Modes
Old 11-16-2018, 11:49 AM   #1
pressplay
Human being with feelings
 
pressplay's Avatar
 
Join Date: Sep 2017
Location: Berlin
Posts: 47
Default Messing with control values from audio thread

The Draw methods of standard IControls do not seem to have any locking mechanism to prevent bad things from happening when members are modified from the audio thread. I understand that we do not want to block the audio thread more than necessary. But if we set mValue of a control from the audio thread - which is encouraged by the API - and the GUI is redrawn at the same time, we have a race condition.



1. Is my observation correct?
2. Or is this prevented on a higher level (the host)?
3. Or is this accepted, because the event is very unlikely?



Thanks in advance!
pressplay is offline   Reply With Quote
Old 11-16-2018, 12:42 PM   #2
Xenakios
Human being with feelings
 
Xenakios's Avatar
 
Join Date: Feb 2007
Location: Oulu, Finland
Posts: 8,062
Default

If the mValue is a float or double, setting it at the same as reading it doesn't cause data corruption on common CPU architectures. No data corruption of course doesn't necessarily mean "what I wanted to happen". The execution order could still be wrong to get the desired outcome. The compiler might even remove writes and reads of the variable when optimizing, because it is allowed to. I've seen this happen : I had a variable for progress monitoring of a long for loop operation happening on another thread. The GUI was reading that variable but only showed full progress all the time with release builds of the code. The compiler had determined that only the last step of the for loop made a meaningful change into that variable and just wrote into it once.

It is still detected as a race condition with the most strict analysis tools, though. So technically I suppose an atomic variable should be used.
__________________
I am no longer part of the REAPER community. Please don't contact me with any REAPER-related issues.

Last edited by Xenakios; 11-16-2018 at 12:54 PM.
Xenakios is offline   Reply With Quote
Old 11-16-2018, 01:20 PM   #3
pressplay
Human being with feelings
 
pressplay's Avatar
 
Join Date: Sep 2017
Location: Berlin
Posts: 47
Default

Quote:
Originally Posted by Xenakios View Post
If the mValue is a float or double, setting it at the same as reading it doesn't cause data corruption on common CPU architectures.

I would really like to believe this... How do you know?
pressplay is offline   Reply With Quote
Old 11-16-2018, 03:00 PM   #4
Xenakios
Human being with feelings
 
Xenakios's Avatar
 
Join Date: Feb 2007
Location: Oulu, Finland
Posts: 8,062
Default

edit : hmm, the float version just caused 28 errors with a different random seed. There could be some issue with how the numbers are checked or there genuinely were errors during the execution.

Obviously anecdotal evidence but the following does not cause the error count to become non-zero. Tested on Visual Studio 2017, release build for 64 bit. Your mileage may vary. Maybe it doesn't work with 32 bit builds, for example.

Code:
template<typename FloatType>
void test_threadloadstore(int64_t iterations)
{
	std::mt19937 gen;
	FloatType minlimit = 0.1;
	FloatType maxlimit = 0.9;
	std::uniform_real_distribution<FloatType> dist(minlimit, maxlimit);
	FloatType observe = 0.5; 
	int64_t errors = 0;
	auto task = [&observe,&errors, iterations, minlimit, maxlimit]()
	{
		for (int64_t i = 0; i < iterations; ++i)
		{
			FloatType x = observe;
			if ((x<minlimit || x>=maxlimit) || std::fpclassify(x)!=FP_NORMAL)
				++errors;
		}
	};
	std::thread th(task);
	for (int64_t i = 0; i < iterations; ++i)
	{
		FloatType x = dist(gen);
		observe = x;
	}
	th.join();
	std::cout << errors << " errors when reading value\n";
}

int main()
{
	test_threadloadstore<float>(1000000000);
	test_threadloadstore<double>(1000000000);
}
If it's not clear from the code itself, it repeatedly generates random numbers in the main thread within a given range and writes those into a variable. The other launched thread repeatedly checks that same variable that it is within the expected bounds and is not a "non-normal" number.
__________________
I am no longer part of the REAPER community. Please don't contact me with any REAPER-related issues.

Last edited by Xenakios; 11-16-2018 at 03:09 PM.
Xenakios is offline   Reply With Quote
Old 11-17-2018, 06:37 AM   #5
pressplay
Human being with feelings
 
pressplay's Avatar
 
Join Date: Sep 2017
Location: Berlin
Posts: 47
Default

I could not produce any errors with this. But I'm not totally conviced yet. What about the following scenario:

MSBs are read first, then the main thread writes new data, then the LSBs are read. You would not detect this data corruption, because you only check if 'observe' is within a valid range. You would get good looking garbage values. Is this realistic or do CPUs just don't work that way?

Besides, the situation could get much worse, if we have two concurrent writes (e.g. audio thread vs mouse event handling).
I don't want to sound paranoid, I'm just motivated to fully understand all the dirty details. Thank you
pressplay is offline   Reply With Quote
Old 11-19-2018, 03:22 PM   #6
pressplay
Human being with feelings
 
pressplay's Avatar
 
Join Date: Sep 2017
Location: Berlin
Posts: 47
Default

On second thought, I think the main problem may not be corrupt data, but not synchronized data. For example the write operation is not seen by another thread which reads from an outdated cache instead. The way to avoid this would be an atomic variable.
Code:
mValue.load(std::memory_order_relaxed);
would probably not introduce a big performance penalty.
pressplay is offline   Reply With Quote
Reply

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump


All times are GMT -7. The time now is 10:23 AM.


Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2024, vBulletin Solutions Inc.