gnulib and threaded execution

[Ralf Wildenhues]

The recent coreutils sort bug related to threading made me take a look
at gnulib for similar issues.  When you try to use gnulib in threaded
code, any process-global state can potentially cause problems, whether
that be static data, file descriptor state, current directory, umask,
etc.  For a lot of these data and in a lot of the cases, gnulib is safe.
Still, it might make sense to make an effort to document exceptions to
this rule: users might not even be aware that three levels down the
module dependency tree, they are using something potentially unsafe.

I egrepped for '([       ]static |static [^()]*;)' (TAB inside) and
among the first few hits (of a few hundred) I found these issues:

* Unless STACK_DIRECTION is defined, gnulib/lib/alloca.c sets and uses
STACK_DIR and find_stack_direction:addr in the first alloca call.  When
that first call is from threads, and racing with another one, the value
for STACK_DIRECTION may be computed wrongly, and the code may corrupt
the stack.

This is an issue only when the compiler/libc doesn't provide it, and
even then, the race seems unlikely to be won, so it's not surprising we
haven't seen a report.  Still, this particular issue could be fixed by
computing STACK_DIRECTION from configure iff we choose to compile
alloca.c.

* error_at_line.c has a function-static old_file_name, such that if
error_at_line is called concurrently with a status of 0, the global
error_one_per_line is set to nonzero, and one of the calls passes NULL
as file_name, then NULL may be passed to strcmp in the other thread, if
the race is won.  Less severely, the old_line_number static could cause
the wrong number to be compared, and since it's an int rather than a
sig_atomic_t, in theory it could even contain an inconsistent value at
times.

Arguably, this use case is pretty contrived, since calling error_at_line
concurrently doesn't mesh with wanting only one error per line, but
IMVHO it makes sense to at least document the requirement for the caller
here.

* in getloadavg.c, getloadavg_initialized should probably be a
sig_atomic_t not a bool (no idea whether this can ever be a problem in
practice[1]).

* register_close_hook is not thread-safe.  Doesn't seem a big issue at
first, it's called only from gl_sockets_startup, but there is no hint
attached to the latter function about not being callable from a threaded
context.


I'm sure a number of other issues are lurking there.  The increased use
of LTO (link-time optimization) will surely over time expose more of
these issues (also things like missing volatile) due to the compiler
being able to optimize much more aggressively.

Cheers,
Ralf