On 11/16/2017 09:13 PM, Eric Blake wrote:
We can't guarantee what storage duration the caller's request for a thread name has; and in fact, if the caller uses plugin_name() for their thread name, then the moment .unload is called, our threadlocal storage is pointing to la-la-land and we get a nice 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 after unload. */ + if (threadlocal) { + free (threadlocal->name); + threadlocal->name = strdup (name); + if (threadlocal->name == NULL) { + perror ("malloc"); + exit (EXIT_FAILURE); + }
I'm thinking (especially since I plan to create threads for each connection) that an exit() may be a bit harsh here (exit during startup is fine; but this code can be reached on more than just startup, and once we transition into accepting connections, we should make reasonable efforts to avoid killing off existing connections with an exit). Thus, I'm tweaking this function to just state that it is best-effort; if strdup() fails, we're better off printing debug messages without a name at all than we are exit()ing. (Then again, if strdup() fails, most likely something else will fail soon anyways). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org