Comment 5 for bug 965798

Revision history for this message
Sven Gothel (sgothel) wrote :

Thank you Chris for responding and describing the history and motivation of the patch.

Regarding visibility, I read in the same tls paper, that global exposes the tls field[s]
to the 'calling' library and executables as well, where the other models don't.
Hence I would be able to access them directly, shortcutting the API.

Sine I am very familiar w/ JOGL's multithreading and did a thorough analysis of
the locking as well, I can almost guarantee - even though you never can be 100% certain,
that at least in this case we have no race condition.
I investigated the call trace w/ manual instrumentation (read: inserted fprintf(stderr, ..))
and the call sequence incl. their thread origin is as expected and good.
Besides, we do run lots of concurrent unit tests in our automated unit tests via jenkins
on different platforms and machines, where none leads to a deadlock or starvation.
This includes GL drivers of NV, AMD, IMG, AMD, .. and Mesa as well ofc.

In this analyzed case, we only have 3 active threads running.
T1 (Main-Thread), T2 (SharedResource-Thread) and T3 (AWT-EventDispatchThread EDT).
At the time T3 issues the 'freezing' context release, T2 and T1 are on the wait queue
doing nothing and also do not hold a current GL context.
This is described in my comment 1.
I am very confident here - the threads in this case are running sequentially,
impossible to cause a race condition.
BTW we also manage our Java side GLContext object, e.g. have Java side locking management implemented.

Even though I don't understand the root cause of the bug, due to my lack of knowledge
about of the actual TLS assembler code - loading libGL on the 'early most' thread (T3 here) allows
proper execution of GLX on it. Hence it makes a difference on which thread the native library gets initialized.
I will prepare an easy to reproduce test case later.

I appreciate your reevaluation of the tls-global patch and hope it gets 'aligned' with vanilla Mesa,
which doesn't have it included. We may then be able to remove our workaround.

Great stuff, thank you very much.