[PATCH libX11] Always initialise thread support

[Jamey Sharp]

On Fri, Jul 12, 2013 at 11:16:16PM +0100, Daniel Stone wrote:
> On 12 July 2013 22:40, Jamey Sharp <jamey at minilop.net> wrote:
> > Hmm. XInitThreads wasn't designed to be used this way. For instance,
> > initializing thread support isn't thread-safe, for fairly obvious
> > reasons.
> >
> > This patch might mask more bugs than it causes, and thereby be a net
> > win. But it seems equally likely to turn out the other way.
> >
> > I'd suggest an awful lot of caution here.
> 
> Hmm.  So the failure mode here would be n threads both calling
> XOpenDisplay simultaneously, all using the Displays independently
> (i.e. never mixing threads) and not having called XInitThreads.  If
> XInitThreads gets entered simultaneously between the test and
> assignment of _Xglobal_lock, then we'll leak n - 1 copies of all the
> mutexes.  The real disaster is if pthread_mutex_init isn't thread-safe
> (non-atomic pointer stores?), in which case the mutex pointers are
> going to be half of one and half of the other.
> 
> So there's a theoretical API break for that case, but that can also be
> fixed by calling XInitThreads beforehand, which I think is acceptable.

I mostly agree with your reasoning, except the subtleties of memory
ordering semantics make this even more murky. Calling XInitThreads first
only helps if it's followed by a write barrier, and the later calls are
preceded by read barriers.

I think pthread_create should have the right barrier semantics in the
parent and child threads, so if you call XInitThreads before
pthread_create, it should be fine--but POSIX apparently doesn't specify
anything about that.

> The immediate provocation was the Mali GLES/EGL implementation, which
> uses multiple threads itself, and thus relies on XInitThreads having
> been called somewhere; so if you ever use that specific
> implementation, every app has to call XInitThreads first to ensure it
> doesn't die horribly.

Yeah, I won't argue that XInitThreads was ever a good idea... :-/ For
that matter, I'm not certain why --disable-xthreads is still allowed.

In fact, I don't really mean to argue against the patch. I'm just
proposing caution.

Jamey
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20130712/61a735ee/attachment.pgp>