search for: threadlocal_set_name

...t; > https://www.redhat.com/archives/libguestfs/2017-September/msg00226.html Running test-socket-activation in a loop, I've hit other races (some provoked a bit more easily with the code I'm working on), and will be posting some patches where I know the solution: Right now, our use of threadlocal_set_name (plugin_name ()) makes our thread-local storage point to a string in module memory. If we have any nbdkit_debug() or other statement that prints after .unload, we get a SEGV. Solution(s): initializing a plugin should strdup() the name so that plugin_name() is valid for the life of the program, rat...

...(+), 2 deletions(-) diff --git a/src/connections.c b/src/connections.c index 921a5b2..e1ac543 100644 --- a/src/connections.c +++ b/src/connections.c @@ -221,7 +221,7 @@ _handle_single_connection (int sockin, int sockout) if (backend->open (backend, conn, readonly) == -1) goto done; - threadlocal_set_name (backend->name (backend)); + threadlocal_set_name (backend->plugin_name (backend)); /* Handshake. */ if (negotiate_handshake (conn) == -1) @@ -253,7 +253,7 @@ _handle_single_connection (int sockin, int sockout) goto wait; } if (asprintf (&worker->name, -...

...ce SEGV while trying to print any debug message. > So copy the user's string instead. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > src/threadlocal.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > @@ -104,8 +105,16 @@ threadlocal_set_name (const char *name) > { > struct threadlocal *threadlocal = pthread_getspecific (threadlocal_key); > > - if (threadlocal) > - threadlocal->name = name; > + /* Copy name, as the original may be residing in a module, but we > + * want our thread name to persist even...

These patches should be ready to go in now; I will also post my work-in-progress for enabling full parallel handling that depends on these, but with that series, I was still getting crashes or hangs with test-socket-activation (I think I've nailed all the crashes I've seen, but the hang is rather insidious; see my other email

...num; /* Can be 0. */ struct sockaddr *addr; socklen_t addrlen; @@ -69,6 +69,7 @@ free_threadlocal (void *threadlocalv) { struct threadlocal *threadlocal = threadlocalv; + free (threadlocal->name); free (threadlocal->addr); free (threadlocal); } @@ -104,8 +105,16 @@ threadlocal_set_name (const char *name) { struct threadlocal *threadlocal = pthread_getspecific (threadlocal_key); - if (threadlocal) - threadlocal->name = name; + /* Copy name, as the original may be residing in a module, but we + * want our thread name to persist even after unload. */ + if (threadloca...

This fixes the race conditions for me, using the test described here: https://www.redhat.com/archives/libguestfs/2017-September/msg00226.html Rich.

...server/sockets.c | 12 ++---------- server/threadlocal.c | 19 ------------------- 3 files changed, 2 insertions(+), 33 deletions(-) diff --git a/server/internal.h b/server/internal.h index 1f72b01..c31bb34 100644 --- a/server/internal.h +++ b/server/internal.h @@ -431,10 +431,6 @@ extern void threadlocal_set_name (const char *name) extern const char *threadlocal_get_name (void); extern void threadlocal_set_instance_num (size_t instance_num); extern size_t threadlocal_get_instance_num (void); -extern void threadlocal_set_sockaddr (const struct sockaddr *addr, - socklen...

...ions (int *socks, size_t nr_socks) + __attribute__((__nonnull__ (1))); +extern void free_listening_sockets (int *socks, size_t nr_socks) + __attribute__((__nonnull__ (1))); /* threadlocal.c */ extern void threadlocal_init (void); extern void threadlocal_new_server_thread (void); -extern void threadlocal_set_name (const char *name); +extern void threadlocal_set_name (const char *name) + __attribute__((__nonnull__ (1))); extern void threadlocal_set_instance_num (size_t instance_num); -extern void threadlocal_set_sockaddr (const struct sockaddr *addr, socklen_t addrlen); +extern void threadlocal_set_sockadd...

I'm fairly sure that these bugs which appear in the Python plugin: https://bugzilla.redhat.com/show_bug.cgi?id=1566516 https://bugzilla.redhat.com/show_bug.cgi?id=1566522 are really bugs in the SERIALIZE_ALL_REQUESTS thread model. See the first patch for the full explanation. The second patch is a fix for a race condition which is probably nudged into being by the first patch. Now this

v1 was here: https://www.redhat.com/archives/libguestfs/2019-January/msg00008.html In v2 I have provided two patches: The first patch extends attribute((nonnull)) to most internal functions, but not to the external API. The second patch uses a macro so that attribute((format)) is only used in the public API on GCC or Clang. At least in theory these headers could be used by a C compiler which

...odel (backend) < NBDKIT_THREAD_MODEL_PARALLEL || + nworkers == 1) nworkers = 0; conn = new_connection (sockin, sockout, nworkers); if (!conn) goto done; - if (plugin_open (conn, readonly) == -1) + if (backend->open (backend, conn, readonly) == -1) goto done; - threadlocal_set_name (plugin_name ()); + threadlocal_set_name (backend->name (backend)); /* Handshake. */ if (negotiate_handshake (conn) == -1) @@ -251,7 +252,8 @@ _handle_single_connection (int sockin, int sockout) set_status (conn, -1); goto wait; } - if (asprintf (&worke...

...odel (backend) < NBDKIT_THREAD_MODEL_PARALLEL || + nworkers == 1) nworkers = 0; conn = new_connection (sockin, sockout, nworkers); if (!conn) goto done; - if (plugin_open (conn, readonly) == -1) + if (backend->open (backend, conn, readonly) == -1) goto done; - threadlocal_set_name (plugin_name ()); + threadlocal_set_name (backend->name (backend)); /* Handshake. */ if (negotiate_handshake (conn) == -1) @@ -251,7 +252,8 @@ _handle_single_connection (int sockin, int sockout) set_status (conn, -1); goto wait; } - if (asprintf (&worke...

...odel (backend) < NBDKIT_THREAD_MODEL_PARALLEL || + nworkers == 1) nworkers = 0; conn = new_connection (sockin, sockout, nworkers); if (!conn) goto done; - if (plugin_open (conn, readonly) == -1) + if (backend->open (backend, conn, readonly) == -1) goto done; - threadlocal_set_name (plugin_name ()); + threadlocal_set_name (backend->name (backend)); /* Handshake. */ if (negotiate_handshake (conn) == -1) @@ -251,7 +252,8 @@ _handle_single_connection (int sockin, int sockout) set_status (conn, -1); goto wait; } - if (asprintf (&worke...

I want to make my nbd forwarding plugin fully parallel - but to do that, I first need to make nbdkit itself fully parallel ;) With this series, I was finally able to demonstrate out-of-order responses when using qemu-io (which is great at sending back-to-back requests prior to waiting for responses) coupled with the nbd file plugin (which has a great feature of rdelay and wdelay, to make it

v1 -> v2: - Fixed everything mentioned in the review. Rich.

Somewhat invasive but mostly mechanical change to how plugins are called. This patch is in preparation for adding a second backend subtype for filters. Rich.

I've posted some of these patches or ideas before; but now I'm confident enough with the series that it should be ready to push; at any rate, I can now run test-socket-activation in a tight loop without triggering any crashes or hangs. With this in place, I'm going back to work on making the nbd forwarder wort with the parallel thread model. Eric Blake (8): sockets: Use

We noticed while writing various libnbd tests that when the delay filter is in use, there are scenarios where we had to resort to SIGKILL to get rid of nbdkit, because it was non-responsive to SIGINT. I'm still trying to figure out the best way to add testsuite coverage of this, but already proved to myself that it works from the command line, under two scenarios that both used to cause long

The first three patches are identical to: https://www.redhat.com/archives/libguestfs/2018-January/msg00079.html "[PATCH nbdkit v2 0/3] Refactor plugin_* functions into a backend" The rest of the patches add filters using the new filter API previously described here: https://www.redhat.com/archives/libguestfs/2018-January/msg00073.html This needs a lot more testing -- and tests --

...e set to NULL at + /* NB: because of an asynchronous exit top can be set to NULL at * just about any time. */ - if (backend) - plugin_name = backend->plugin_name (backend); + if (top) + plugin_name = top->plugin_name (top); else plugin_name = "(unknown)"; threadlocal_set_name (plugin_name); - if (backend && backend->preconnect (backend, read_only) == -1) + if (top && top->preconnect (top, read_only) == -1) goto done; /* NBD handshake. @@ -225,7 +225,7 @@ handle_single_connection (int sockin, int sockout) /* Finalize (for filters)...