bug#36609: 27.0.50; Possible race-condition in threading implementation

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Date: Sat, 13 Jul 2019 14:37:25 +0000
> Cc: 36609@debbugs.gnu.org, politza@hochschule-trier.de
> 
> Well, I think we're going to have to do one or more of the following:
> 
> - have a race condition
> - access glib-locked data from the "wrong" thread (another Emacs thread)
> - release the glib lock from the "wrong" thread (another Emacs thread)
> 
> Of these, the second is the best alternative, I think: we simply grab
> the g_main_context lock globally, acting for all Emacs threads, and
> the last thread to require it releases it when it leaves xg_select.

I agree.

> As long as there's at least one thread in the critical section of
> xg_select, we hold the lock, but access to the context isn't
> necessarily from the thread which locked it.

Hmm... wouldn't this cause us always hold that lock when there's more
than one Lisp thread?  And if so, what will that do to GTK and its own
threads, which also want to acquire the Glib context?

IOW, I guess I don't understand what problem would the proposed
changes solve that the current code doesn't.  Can you tell?

> > Hmm... how would this work with your patch?  Suppose one thread calls
> > xg_select, acquires the Glib lock, sets its holding_glib_lock flag,
> > then releases the global Lisp lock and calls pselect.  Since the
> > global Lisp lock is now up for grabs, some other Lisp thread can
> > acquire it and start running.
> 
> And when it starts running, it releases the Glib lock.

But only if its holding_glib_lock flag is set, which is not
necessarily true.  For example, what about a thread that was just
created, and never called xg_select yet?

> Okay, that sounds like option #2 above. The attached patch exposes
> glib externals to the generic code, but it appears to work. If you
> think the approach is okay, I'll move the glib-specific parts to
> xgselect.c (if that's okay).

Yes, we should have accessor functions in xgselect.c instead of
letting thread.c etc. access the GTK/Glib data directly.

But I still would like to understand why we need to maintain a flag
per thread.

Also, I think I lost the context: does this attempt to solve the
original problem reported by Andreas, or only the problem reported by
you later in the discussion?

Thanks.