On Wed, Jan 17, 2018 at 08:15:05AM +0000, Wuchongyun wrote: > Hi Matin, > I think it's a good idea to move the close(ux_sock) call further up to avoid > new clients trying to connect, Below is the new patch for your comment. > Please help to review. Thanks.
Looks good. Reviewed-by: Benjamin Marzinski <[email protected]> > Issue description: we meet this issue: when multipathd initilaze and > call uxsock_listen to create unix domain socket, but return -1 and > the errno is 98 and then the uxsock_listen return null. After multipathd > startup we can't receive any user's multipathd commands to finish the > new multipath creation or any operations any more! > > We found that uxlsnr thread's cleanup function not close the sockets > also not release the clients when cancel thread, the domain socket > will be release by the system. In any special environment like the > machine's load is very heavy or any situations, the system may not close > the old domain socket when we try to create and bind the new domain > socket may return errno:98(Address already in use). > > And also we make some experiments: > in uxsock_cleanup if we close the ux_sock first and then immdediately > call ux_socket_listen to create new ux_sock and initialization will be > OK; if we don't close the ux_sock and call ux_socket_listen will return > -1 and errno = 98. > > So we believe that close uxsocket and release clients when cancel > thread can make sure of that new starting multipathd thread can > create new uxsocket successfully, also can receive multipathd commands > properly. And this path can fix clients' memory leak too. > > Signed-off-by: Chongyun Wu <[email protected]> > --- > multipathd/uxlsnr.c | 30 ++++++++++++++++++++++++------ > 1 file changed, 24 insertions(+), 6 deletions(-) > > diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c > index 98ac25a..8aa0f83 100644 > --- a/multipathd/uxlsnr.c > +++ b/multipathd/uxlsnr.c > @@ -102,14 +102,21 @@ static void new_client(int ux_sock) > /* > * kill off a dead client > */ > -static void dead_client(struct client *c) > +static void _dead_client(struct client *c) > { > - pthread_mutex_lock(&client_lock); > + int fd = c->fd; > list_del_init(&c->node); > - pthread_mutex_unlock(&client_lock); > - close(c->fd); > c->fd = -1; > FREE(c); > + close(fd); > +} > + > +static void dead_client(struct client *c) > +{ > + pthread_cleanup_push(cleanup_lock, &client_lock); > + pthread_mutex_lock(&client_lock); > + _dead_client(c); > + pthread_cleanup_pop(1); > } > > void free_polls (void) > @@ -139,6 +146,18 @@ void check_timeout(struct timespec start_time, char > *inbuf, > > void uxsock_cleanup(void *arg) > { > + struct client *client_loop; > + struct client *client_tmp; > + int ux_sock = (int)arg; > + > + close(ux_sock); > + > + pthread_mutex_lock(&client_lock); > + list_for_each_entry_safe(client_loop, client_tmp, &clients, node) { > + _dead_client(client_loop); > + } > + pthread_mutex_unlock(&client_lock); > + > cli_exit(); > free_polls(); > } > @@ -162,7 +181,7 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, > void * trigger_data) > return NULL; > } > > - pthread_cleanup_push(uxsock_cleanup, NULL); > + pthread_cleanup_push(uxsock_cleanup, (void *)ux_sock); > > condlog(3, "uxsock: startup listener"); > polls = (struct pollfd *)MALLOC((MIN_POLLS + 1) * sizeof(struct > pollfd)); > @@ -300,6 +319,5 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, > void * trigger_data) > } > > pthread_cleanup_pop(1); > - close(ux_sock); > return NULL; > } > -- > 1.7.9.5 -- dm-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/dm-devel
