Re: [PATCH 1/3] Make several more BFD globals thread-local

[Nick Clifton <nickc@redhat.com>]

Hi Tom,

> Among other things, PR gdb/31264 points out a race in bfd_check_format
> -- it sets the error handler, which is a global.
> 
> Looking into this a bit more, I found several other possible races:
> the "in_check_format" static local variable in
> bfd_check_format_matches, and the contents of per_xvec_warn.
> 
> This patch makes all of these thread-local.
> 
> I don't actually think this is the best way to approach this.
> "in_check_format" and the per-xvec warnings could be done by setting a
> flag on the BFD, avoiding globals entirely.  I can do that if you
> want; I just wasn't sure if that is desirable or not.

Actually, I think that I would prefer this alternative solution.

I am particularly worried about making the error handler thread local
when this might not match the expectations of a client using the BFD
library.  Since the error handler is currently global, a client can
expect to set it once and have it affect all threads.

Looking at the code in bfd_check_format_matches() it seems clear that
the error handler manipulation is really a hack, based upon the single
threaded nature of the BFD library.  This become problematical when the
BFD library is used in a multi-threaded environment.

It seems to me that the proper thing to do would be to [throw away the
BFD library and use something designed from the ground up to thread-safe],
ahem, I mean provide a thread safe way for bfd_check_format_matches to
override the error handler within its local context, without it affecting
the global error handler for other threads *and* still allowing the client
to call bfd_set_error_handler to change the error handler for all threads
at once.

This sounds like a lot of work however, and probably something that
ought to be done as part of a larger project to turn the BFD library
into a thread-safe and multi-threaded library.

What are you thoughts ?

Cheers
   Nick