[alsa-devel] [PATCH 0/2] Fix thread safety issues

[Jérôme Forissier]

On 31 janv. 2013, at 09:54, Takashi Iwai wrote:

> At Wed, 30 Jan 2013 19:13:54 +0100,
> Jérôme Forissier wrote:
>> 
>> On 30 janv. 2013, at 17:03, Takashi Iwai wrote:
>> 
>>> At Wed, 30 Jan 2013 16:22:15 +0100,
>>> Jerome Forissier wrote:
<...>
>>> snd_device_name_hint() should use snd_config_update_r() and pass the
>>> local config space to others instead of snd_config.
>> 
>> I tried following this path, but there is a problem: try_config() calls snd_lib_error_set_handler() so I fail to see how we can make this code reentrant.
>> In fact, the random errors I've seen when running my multi-threaded test were probably just a side-effect of fiddling with the global error handler from multiple threads. Probably harmless, but still it makes snd_device_name_hint() not thread-safe, strictly speaking.
> 
> Yeah, I see the issue now, too.
> 
> The problem is that the error handler is really global, not only used
> in conf.c and co.  Thus applying a lock locally doesn't suffice.
> 
>> Any suggestion? Should I consider moving snd_lib_error to thread-local storage?
> 
> TLB would be an option, indeed.  But blindly moving snd_lib_error to
> TLS breaks the current behavior, so we should avoid it.

One could argue that the current behavior with multi-threaded apps is undefined anyways. But I see what you mean.

> How about a patch like below?  It adds another error redirection.
> Unless local_error is set, the behavior is identical.

Nice suggestion. Will try that. After all, "all problems in computer science can be solved by another level of indirection..." ;-)

> (Maybe we can name it better, yeah.  I just wanted to avoid to put a
> word "thread" in the function because it should be usable in both
> thread- and non-thread-capable environment.)

Well you can consider that, in a non-threaded environment, there is just one thread (the main thread). 

Thanks for your help. I will post an updated patch when done.

-- 
Jerome