On 2023-09-14 09:42, Tomáš Golembiovský wrote: > This is another take on the ancient saga of closing sockets from one > thread while another thread is blocked on recv(2). It all started with > [1,2] and continued by [3]. It was established that this is expected and > long term behavior or Linux and the issue was closed by Michael Kerrisk > by commit c2f15a1349a7271f6c1d81361ec8b256266e1c09. > > This is all fine and the patch covered the issue in general terms. > However, it does not mention the specific case of sockets and shutdown, > where the issue can be (at least for the read case) mitigated by proper > shutdown. While one may argue that such information may be implied by > other man pages (perhaps return value of recv(2)) and thus is redundant, > it seems only fair to mention shutdown(2) here as it is only rarely > noted in Linux documentation that properly shutting down both side of > the socket is a good programming practice when dealing with sockets. > > As a test program I am attaching the program originally written by Lukas > Czerner. Only with small fixes here and there. > > [1] https://lore.kernel.org/linux-man/1314620800-15587-1-git-send-email-lczerner@xxxxxxxxxx/ > [2] https://bugzilla.redhat.com/show_bug.cgi?id=650985 > [3] https://bugzilla.kernel.org/show_bug.cgi?id=53781 > > ```c > /** > * Copyright Red Hat > * SPDX-License-Identifier: GPL-2.0-or-later > */ > > void *close_socket(void *arg) { > int sockfd = *(int *)arg; > > sleep(3); > printf("Thread: closing socket %d\n", sockfd); > // shutdown(sockfd, SHUT_RDWR); > close(sockfd); > return NULL; > } > > int client(void) { > int sockfd; > int len; > struct sockaddr_un address; > int ret; > char *buffer=malloc(BUFSIZE); > pthread_t thread; > > sockfd = socket(AF_UNIX, SOCK_STREAM, 0); > > address.sun_family = AF_UNIX; > strcpy(address.sun_path, "server_socket"); > len = sizeof(address); > > ret = connect(sockfd, (struct sockaddr *)&address, len); > if (ret == -1) { > perror("connect"); > return 1; > } > printf("client connected\n"); > > ret = pthread_create(&thread, NULL, close_socket, (void *)&sockfd); > if (ret != 0) { > perror("Creating thread failed"); > return 1; > } > > while (1) { > ret = recv(sockfd,buffer,BUFSIZE,0); > if (ret < 0) { > perror("recv"); > return 1; > } > printf("Data received: %s\n", buffer); > sleep(1); > } > > close(sockfd); > return 0; > } > > int server(void) { > char *message="This is the message I am sending to you"; > struct sockaddr_un server_addr, client_addr; > int server_sockfd, client_sockfd; > socklen_t server_len, client_len; > int ret; > > unlink("server_socket"); > server_sockfd = socket(AF_UNIX, SOCK_STREAM, 0); > > server_addr.sun_family = AF_UNIX; > strcpy(server_addr.sun_path, "server_socket"); > server_len = sizeof(server_addr); > bind(server_sockfd, (struct sockaddr *)&server_addr, server_len); > > listen(server_sockfd, 5); > > client_len = sizeof(client_addr); > client_sockfd = accept(server_sockfd, > (struct sockaddr *)&client_addr, &client_len); > > printf("Server: sending data...\n"); > ret = send(client_sockfd ,message,strlen(message),0); > if (ret < 0) { > perror("send"); > return 1; > } > > /* simulate running server by not closing the client_socket socket */ > return 0; > } > > int main() { > pid_t pid; > > pid = fork(); > if (pid < 0) { > perror("fork failed"); > exit(1); > } > if (pid > 0) { > printf(" - starting server\n"); > server(); > printf(" - exiting server\n"); > wait(NULL); > } else { > sleep(1); > printf(" - starting client\n"); > client(); > printf(" - exiting client\n"); > } > } > ``` Here are some fixes to the program: $ diff -u old.c new.c --- old.c 2023-09-14 11:35:06.556184223 +0200 +++ new.c 2023-09-14 11:35:33.109203789 +0200 @@ -3,6 +3,16 @@ * SPDX-License-Identifier: GPL-2.0-or-later */ + #include <pthread.h> + #include <stddef.h> + #include <stdio.h> + #include <stdlib.h> + #include <string.h> + #include <sys/socket.h> + #include <sys/un.h> + #include <sys/wait.h> + #include <unistd.h> + void *close_socket(void *arg) { int sockfd = *(int *)arg; @@ -15,10 +25,11 @@ int client(void) { int sockfd; - int len; + socklen_t len; struct sockaddr_un address; int ret; - char *buffer=malloc(BUFSIZE); + ssize_t r; + char *buffer=malloc(BUFSIZ); pthread_t thread; sockfd = socket(AF_UNIX, SOCK_STREAM, 0); @@ -41,8 +52,8 @@ } while (1) { - ret = recv(sockfd,buffer,BUFSIZE,0); - if (ret < 0) { + r = recv(sockfd, buffer, BUFSIZ, 0); + if (r < 0) { perror("recv"); return 1; } @@ -59,7 +70,7 @@ struct sockaddr_un server_addr, client_addr; int server_sockfd, client_sockfd; socklen_t server_len, client_len; - int ret; + ssize_t ret; unlink("server_socket"); server_sockfd = socket(AF_UNIX, SOCK_STREAM, 0); @@ -76,7 +87,7 @@ (struct sockaddr *)&client_addr, &client_len); printf("Server: sending data...\n"); - ret = send(client_sockfd ,message,strlen(message),0); + ret = send(client_sockfd, message, strlen(message), 0); if (ret < 0) { perror("send"); return 1; After those fixes, I still get a warning of dead code: $ clang -Weverything -Wno-missing-prototypes new.c new.c:64:9: warning: code will never be executed [-Wunreachable-code] close(sockfd); ^~~~~ 1 warning generated. > > Signed-off-by: Tomáš Golembiovský <tgolembi@xxxxxxxxxx> > --- > > v2: updated test file header > > man2/close.2 | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/man2/close.2 b/man2/close.2 > index 68211bc58..08c6a0839 100644 > --- a/man2/close.2 > +++ b/man2/close.2 > @@ -145,6 +145,11 @@ Thus, the blocking system call in the first thread may successfully > complete after the > .BR close () > in the second thread. > +.PP > +When dealing with sockets, > +blocking forever in another thread may be prevented by using > +.BR shutdown (2) > +to shut down both parts of the connection before closing the socket. > .\" > .SS Dealing with error returns from close() > A careful programmer will check the return value of -- <http://www.alejandro-colomar.es/> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
Attachment:
OpenPGP_signature.asc
Description: OpenPGP digital signature
