Re: [Libguestfs] [PATCH 3/5] threads: Use thread-local storage for errors.

On Sat, Jun 06, 2015 at 02:20:39PM +0100, Richard W.M. Jones wrote: > We permit the following constructs in libguestfs code: > > if (guestfs_some_call (g) == -1) { > fprintf (stderr, "failed: error is %s\n", guestfs_last_error (g)); > } > > and: > > guestfs_push_error_handler (g, NULL, NULL); > guestfs_some_call (g); > guestfs_pop_error_handler (g); > > Neither of these would be safe if we allowed the handle to be used > from threads concurrently, since the error string or error handler > could be changed by another thread. > > Solve this in approximately the same way that libvirt does: by making > the error, current error handler, and stack of error handlers use > thread-local storage (TLS). > > The implementation is not entirely straightforward, mainly because > POSIX doesn't give us useful destructor behaviour, so effectively we > end up creating our own destructor using a linked list. > > Note that you have to set the error handler in each thread separately, > which is an API change (eg: if you set the error handler in one > thread, then pass the handle 'g' to another thread, the error handler > in the second thread appears to have reset itself back to the default > error handler). I haven't yet worked out a better way to solve this. Do you have a feeling whether the error handler function push/pop is a commonly used feature or not ?

In libvirt we originally had virGetLastError (global error) virConnGetLastError (per connection error) virSetErrorFunc (global error handler func) When we introduced thread support we didn't try to make the virConnGetLastError/SetErrorFun work. We just updated the docs to tell applications to /never/ use them in threaded applications, and always use the virGetLastError which was thread-local backed. For back compatibility we do copy the error from the global thread local into the per-connection object, so we didn't break single-threaded apps using that old function. IOW I think you could avoid alot of the complexity here if you just marked the existing error handler functions as deprecated in the context of multi-threaded usage and just introduced a single global "get error" function that was backed by a static allocated thread local.