search for: handle_single_connect

..._options (conn=<optimized out>) at protocol-handshake-newstyle.c:484 #6 protocol_handshake_newstyle (conn=0x55771f1ac710) at protocol-handshake-newstyle.c:762 #7 0x00005577156b5705 in protocol_handshake (conn=conn at entry=0x55771f1ac710) at protocol-handshake.c:55 #8 0x00005577156af73a in handle_single_connection (sockin=<optimized out>, sockout=15) at connections.c:167 #9 0x00005577156b8a48 in start_thread (datav=0x55771f18c620) at sockets.c:356 #10 0x00007f12b885f4e2 in start_thread (arg=<optimized out>) at pthread_create.c:479 #11 0x00007f12b878e643 in clone () at ../sysdeps/unix/sysv/lin...

Commit 876d715e17a59bd25df53be34b942afd96e52da9 ("Refactor plugin_* functions into a backend struct.") causes hidden crashes in the test suite (which unfortunately don't cause tests to fail). These all come from handle_single_connection where we may lock the connection, run a single connection, free the backend, and then try to unlock the connection. Unfortunately the unlock operation has to check the thread model again which fails because the backend has gone away: Program terminated with signal SIGSEGV, Segmentation fault...

...04066ff in filter_free (b=0x203c330) at filters.c:77 #3 0x000000000040a6f4 in main (argc=11, argv=0x7ffc1f4486e8) at main.c:649 Thread 1 (Thread 0x7f1caaa5e700 (LWP 7226)): #0 0x000000000040732a in filter_finalize (b=0x203c330, conn=0x203d870) at filters.c:421 #1 0x0000000000404d07 in _handle_single_connection (sockin=6, sockout=6) at connections.c:239 #2 0x0000000000404d76 in handle_single_connection (sockin=6, sockout=6) at connections.c:258 #3 0x00000000004119f6 in start_thread (datav=0x203b450) at sockets.c:263 #4 0x00007f1cab09b5a2 in start_thread () from /lib64/libpthread.so.0 #...

...read_clockjoin_ex () from /lib64/libpthread.so.0 #1 0x00007fabc090af2b in nbdplug_close_handle (h=0x5584020e09b0) at nbd.c:538 #2 0x00005583f90caee0 in backend_close (b=<optimized out>) at backend.c:247 #3 0x00005583f90cdbf1 in free_connection (conn=0x5584020df890) at connections.c:359 #4 handle_single_connection (sockin=<optimized out>, sockout=<optimized out>) at connections.c:230 #5 0x00005583f90d63e8 in start_thread (datav=0x5584020bf1b0) at sockets.c:356 #6 0x00007fabc069d472 in start_thread () from /lib64/libpthread.so.0 #7 0x00007fabc05cc063 in clone () from /lib64/libc.so.6 Thread...

...are definitely lost in loss record 4 of 10 ==1251605== at 0x896180B: malloc (vg_replace_malloc.c:309) ==1251605== by 0x11909F: protocol_handshake_newstyle (protocol-handshake-news tyle.c:288) ==1251605== by 0x118804: protocol_handshake (protocol-handshake.c:55) ==1251605== by 0x112080: handle_single_connection (connections.c:165) ==1251605== by 0x11B84D: start_thread (sockets.c:276) ==1251605== by 0x8BB74E1: start_thread (pthread_create.c:479) ==1251605== by 0x8CD3642: clone (clone.S:95) I didn't look at it closely but there does appear to be a memory leak in this patch. Rich. -- Rich...

...STDOUT_FILENO) { + perror ("open"); + exit (EXIT_FAILURE); + } + /* Select the correct thread model based on config. */ lock_init_thread_model (); @@ -918,7 +954,7 @@ start_serving (void) change_user (); write_pidfile (); threadlocal_new_server_thread (); - handle_single_connection (0, 1); + handle_single_connection (orig_in, orig_out); return; } -- 2.26.0

...STDOUT_FILENO) { + perror ("open"); + exit (EXIT_FAILURE); + } + /* Select the correct thread model based on config. */ lock_init_thread_model (); @@ -918,7 +949,7 @@ start_serving (void) change_user (); write_pidfile (); threadlocal_new_server_thread (); - handle_single_connection (0, 1); + handle_single_connection (orig_in, orig_out); return; } diff --git a/tests/test-layers-plugin.c b/tests/test-layers-plugin.c index 8858bede..5a6b3020 100644 --- a/tests/test-layers-plugin.c +++ b/tests/test-layers-plugin.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2018-2...

....7.so.1.0 > #9 0x00007ffff73f0af0 in py_close (handle=0x625338) at python.c:234 > #10 0x0000000000405803 in plugin_close (conn=conn@entry=0x7fffe8000910) at plugins.c:377 > #11 0x00000000004037ec in free_connection (conn=0x7fffe8000910) at connections.c:147 > #12 0x0000000000404476 in _handle_single_connection (sockout=<optimized out>, sockin=<optimized out>) at connections.c:99 > #13 handle_single_connection (sockin=<optimized out>, sockout=<optimized out>) at connections.c:109 > #14 0x0000000000405e73 in start_thread (datav=0x7fffffffdd40) at sockets.c:220 > #15 0x00...

...linux-gnu/libpython2.7.so.1.0 #9 0x00007ffff73f0af0 in py_close (handle=0x625338) at python.c:234 #10 0x0000000000405803 in plugin_close (conn=conn@entry=0x7fffe8000910) at plugins.c:377 #11 0x00000000004037ec in free_connection (conn=0x7fffe8000910) at connections.c:147 #12 0x0000000000404476 in _handle_single_connection (sockout=<optimized out>, sockin=<optimized out>) at connections.c:99 #13 handle_single_connection (sockin=<optimized out>, sockout=<optimized out>) at connections.c:109 #14 0x0000000000405e73 in start_thread (datav=0x7fffffffdd40) at sockets.c:220 #15 0x00007ffff79c0184...

...Blake <eblake@redhat.com> --- src/sockets.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/sockets.c b/src/sockets.c index edb63b3..1d63523 100644 --- a/src/sockets.c +++ b/src/sockets.c @@ -261,6 +261,7 @@ start_thread (void *datav) handle_single_connection (data->sock, data->sock); + free (data); return NULL; } @@ -270,18 +271,25 @@ accept_connection (int listen_sock) int err; pthread_attr_t attrs; pthread_t thread; - struct thread_data thread_data; + struct thread_data *thread_data; static size_t instance_num = 1; - t...

...t; > Thread 1 (Thread 0x7f1caaa5e700 (LWP 7226)): > #0 0x000000000040732a in filter_finalize (b=0x203c330, conn=0x203d870) > at filters.c:421 ...filter_finalize() time to finish its job? But I agree that solving the race is an independent patch. > #1 0x0000000000404d07 in _handle_single_connection (sockin=6, sockout=6) > at connections.c:239 > #2 0x0000000000404d76 in handle_single_connection (sockin=6, sockout=6) > at connections.c:258 > #3 0x00000000004119f6 in start_thread (datav=0x203b450) at sockets.c:263 > #4 0x00007f1cab09b5a2 in start_thread () from...

...oss record 4 of 10 > ==1251605== at 0x896180B: malloc (vg_replace_malloc.c:309) > ==1251605== by 0x11909F: protocol_handshake_newstyle (protocol-handshake-news > tyle.c:288) > ==1251605== by 0x118804: protocol_handshake (protocol-handshake.c:55) > ==1251605== by 0x112080: handle_single_connection (connections.c:165) > ==1251605== by 0x11B84D: start_thread (sockets.c:276) > ==1251605== by 0x8BB74E1: start_thread (pthread_create.c:479) > ==1251605== by 0x8CD3642: clone (clone.S:95) > > I didn't look at it closely but there does appear to be a memory leak > i...

...0000000040685d in nbdkit_debug (fs=fs@entry=0x40ace6 "close") at errors.c:91 #4 0x00000000004077d9 in plugin_close (conn=conn@entry=0x7f679c000910) at plugins.c:386 #5 0x0000000000404de3 in free_connection (conn=0x7f679c000910) at connections.c:228 #6 0x00000000004059fd in _handle_single_connection (sockout=<optimized out>, sockin=<optimized out>) at connections.c:179 #7 handle_single_connection (sockin=<optimized out>, sockout=<optimized out>) at connections.c:189 #8 0x0000000000408023 in start_thread (datav=0x7fff1a4fbbe0) at sockets.c:262 #9 0x00007f6...

This is what I've been playing with in response to my earlier question about what to do with 'nbdkit -s sh -' (https://www.redhat.com/archives/libguestfs/2020-April/msg00032.html) I'm still open to ideas on a better name, and/or whether adding <stdbool.h> to our public include files is a good idea (if not, returning int instead of bool is tolerable). Eric Blake (2):

...addr addr; - socklen_t addrlen; }; static void * @@ -274,7 +272,6 @@ start_thread (void *datav) /* Set thread-local data. */ threadlocal_new_server_thread (); threadlocal_set_instance_num (data->instance_num); - threadlocal_set_sockaddr (&data->addr, data->addrlen); handle_single_connection (data->sock, data->sock); @@ -299,12 +296,9 @@ accept_connection (int listen_sock) } thread_data->instance_num = instance_num++; - thread_data->addrlen = sizeof thread_data->addr; again: #ifdef HAVE_ACCEPT4 - thread_data->sock = accept4 (listen_sock, -...

...+ b/src/Makefile.am @@ -40,6 +40,7 @@ nbdkit_SOURCES = \ crypto.c \ errors.c \ internal.h \ + locks.c \ main.c \ plugins.c \ protocol.h \ diff --git a/src/connections.c b/src/connections.c index 111a810..74bb8e4 100644 --- a/src/connections.c +++ b/src/connections.c @@ -211,7 +211,7 @@ _handle_single_connection (int sockin, int sockout) int nworkers = threads ? threads : DEFAULT_PARALLEL_REQUESTS; pthread_t *workers = NULL; - if (!plugin_is_parallel() || nworkers == 1) + if (plugin_thread_model () < NBDKIT_THREAD_MODEL_PARALLEL || nworkers == 1) nworkers = 0; conn = new_connection...

...+ b/src/Makefile.am @@ -40,6 +40,7 @@ nbdkit_SOURCES = \ crypto.c \ errors.c \ internal.h \ + locks.c \ main.c \ plugins.c \ protocol.h \ diff --git a/src/connections.c b/src/connections.c index 111a810..74bb8e4 100644 --- a/src/connections.c +++ b/src/connections.c @@ -211,7 +211,7 @@ _handle_single_connection (int sockin, int sockout) int nworkers = threads ? threads : DEFAULT_PARALLEL_REQUESTS; pthread_t *workers = NULL; - if (!plugin_is_parallel() || nworkers == 1) + if (plugin_thread_model () < NBDKIT_THREAD_MODEL_PARALLEL || nworkers == 1) nworkers = 0; conn = new_connection...

...ernal.h +++ b/src/internal.h @@ -131,8 +131,8 @@ typedef int (*connection_recv_function) (struct connection *, void *buf, size_t typedef int (*connection_send_function) (struct connection *, const void *buf, size_t len); typedef void (*connection_close_function) (struct connection *); extern int handle_single_connection (int sockin, int sockout); -extern void connection_set_handle (struct connection *conn, void *handle); -extern void *connection_get_handle (struct connection *conn); +extern int connection_set_handle (struct connection *conn, size_t i, void *handle); +extern void *connection_get_handle (struct c...

...ugin and filter. */ size_t nr_handles; char exportname[NBD_MAX_STRING + 1]; @@ -239,6 +247,12 @@ struct connection { connection_close_function close; }; +static inline struct handle * +get_handle (struct connection *conn, int i) +{ + return &conn->handles[i]; +} + extern void handle_single_connection (int sockin, int sockout); extern int connection_get_status (void); extern int connection_set_status (int value); diff --git a/server/backend.c b/server/backend.c index 616c24d8..9669ada1 100644 --- a/server/backend.c +++ b/server/backend.c @@ -154,7 +154,7 @@ int backend_open (struct backend...

...ernal.h +++ b/src/internal.h @@ -134,8 +134,8 @@ typedef int (*connection_recv_function) (struct connection *, void *buf, size_t typedef int (*connection_send_function) (struct connection *, const void *buf, size_t len); typedef void (*connection_close_function) (struct connection *); extern int handle_single_connection (int sockin, int sockout); -extern void connection_set_handle (struct connection *conn, void *handle); -extern void *connection_get_handle (struct connection *conn); +extern int connection_set_handle (struct connection *conn, size_t i, void *handle); +extern void *connection_get_handle (struct c...