Re: [PATCH][PR guile/17247] Block SIGCHLD while initializing Guile

[Doug Evans]

Eli Zaretskii <address@hidden> writes:
>> Date: Mon, 1 Sep 2014 15:04:16 -0700
>> From: Doug Evans <address@hidden>
>> Cc: Ludovic Courtès <address@hidden>, 
>>      guile-devel <address@hidden>, 
>>      "address@hidden" <address@hidden>
>> 
>>> Eli Zaretskii skribis:
>>>
>>> > Perhaps we should request GC and Guile to provide capabilities to
>>> > disable threads at run time, then.
>>>
>>> I don’t think we need to go this far: reading the recent discussions, it
>>> seems Doug found a way to make sure Guile’s and libgc’s internal threads
>>> don’t receive signals that GDB is interested in, which should be enough
>>> for practical purposes.
>>
>> That's just the tip of an iceberg, I'm afraid.
>> 
>> In the interests of seeking clarification early and clearly, can you 
>> elaborate?
>> 
>> I'm not saying there aren't issues, I'm just wondering if you have
>> anything specific that you are thinking of.
>
> OK, but if no one except myself sees the danger, then that's about the
> last time I'm writing about this, and will hold my peace thereafter.
>
> With that out of my way...
>
> Basically, anything you can do in Guile that somehow causes
> process-wide changes would present a problem, if Guile threads are
> allowed in GDB.  Here are a few examples that come to mind:
>
>  . redirection of standard descriptors (I already mentioned this in my
>    initial message in this discussion)
>
>  . changing environment variables with setenv/unsetenv
>
>  . changing the current directory with chdir
>
>  . changing the process's umask
>
>  . calling setuid/seteuid or setsid
>
>  . changing the priority or affinity of the process
>
>  . setting or changing signal handlers, or raising signals
>
>  . setting or canceling interval timers
>
>  . changing resource limits with setrlimit
>
> The full list is almost infinite, since Guile supports foreign
> libraries via libffi and libltdl.  So a Guile extension can load any
> code that might call any API.
>
> Since GDB is itself a program that uses at least some of the low-level
> OS interfaces that underly the above Guile functionalities, the danger
> of core GDB and threads running Guile extensions stomping onto each
> other's feet is not just theoretical, IMO.
>
> Moreover, the proposed solutions only address the issues we know about
> at this time.  Both GC and Guile are being actively maintained, so
> there's a real danger they will introduce more features whose use from
> a separate thread will present problems beyond those known today.  I
> predict this to be a constant source of maintainers' headache.  We
> will probably see more incidents like this one, where we released a
> seriously flawed GDB.
>
> We had our share of similar problems in Emacs compiled with GTK, where
> it turned out that GTK launches threads at will, and some of those
> threads manipulate signals and even start subprocesses(!).  There are
> several horror stories in the Emacs bug tracker about related
> hard-to-debug crashes caused, e.g., by GTK usurping SIGCHLD while
> Emacs was waiting for a subprocess to terminate.
>
> In a single-threaded program, a well-behaved Guile extension can at
> least minimize the damage by restoring the global state immediately
> after doing whatever it needed the change in that state for.  But if
> the extension code runs in a separate thread, there's no way of doing
> that safely and reliably.
>
> We could introduce some synchronization mechanism, whereby, for
> example, GDB's main thread could cause all other threads stop in their
> tracks while it is accessing some global resource, and then resumes
> them.  But that means added complexity, and doesn't entirely fix the
> problem (e.g., a chdir call that doesn't return to the original
> directory could still cause harm to core GDB).
>
> So I suggest to think about this now, rather than bearing the
> maintenance burden in the years to come.

I'm happy to leave users to their own devices,
we can't physically prevent them from starting threads.
We pretty much leave them that way already given the myriad
of things they can do to mess up gdb without threads.

The python side of things is too silent on whether threads are supported
there.
https://sourceware.org/gdb/current/onlinedocs/gdb/Basic-Python.html#Basic-Python
The only thing it says with regards to threads is:
"GDB takes care to mark its internal file descriptors as
close-on-exec.  However, this cannot be done in a thread-safe way on
all platforms.  Your Python programs should be aware of this and
should both create new file descriptors with the close-on-exec flag
set and arrange to close unneeded file descriptors before starting a
child process."

At any rate, the task at hand is getting gdb to work well with Guile,
regardless of whether the user starts any threads,
and that's the intent of the patch.
I am happy to make the default value of GC_MARKERS be 1 (regardless
of libgc version, and if not already set by the user) when initializing Guile,
that will disable libgc marker threads.