[alsa-devel] [PATCH 0/2] Fix thread safety issues
Takashi Iwai
tiwai at suse.de
Thu Jan 31 11:09:23 CET 2013
At Thu, 31 Jan 2013 10:48:27 +0100,
Jérôme Forissier wrote:
>
>
> 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.
Yes, it was simply badly designed like errno.
We can't change it at this point, but put a clear statement at least.
If so, the web page should updated, too.
Jaroslav, could you add some texts like below? (Feel free to
rephrase.)
"There are a few global variables that are used in alsa-lib, such as
the global error handler pointer and the global root config pointer.
Accessing / modifying these would be thread-unsafe."
> 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..." ;-)
Hehe.
> > (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).
Hm, one can call it so, too. I don't mind much how it's named.
Spell it if you have a better one.
thanks,
Takashi
>
> Thanks for your help. I will post an updated patch when done.
>
> --
> Jerome
>
More information about the Alsa-devel
mailing list