hiredis icon indicating copy to clipboard operation
hiredis copied to clipboard

Using hiredis in multithread environment causes problems with socket close

Open joppino opened this issue 5 years ago • 0 comments

This is not really a hiredis direct issue, but the way in which sockets are being closed in hiredis library. In the redisFree() function, we have a close() on the fd that's used by redis, then all other resources are free'd:

void redisNetClose(redisContext *c) { if (c && c->fd != REDIS_INVALID_FD) { close(c->fd); c->fd = REDIS_INVALID_FD; } }

Unfortunately, in a multithreaded environment, this is what happens if you free the redisContexts using redisFree() in a different thread than the sockets were created into:

Opened 110 connections to redis. 32289 is our client pid

[root@kushan]# ls /proc/32289/fd 0 100 103 106 11 14 17 2 22 25 28 30 33 36 39 41 44 47 5 52 55 58 60 63 66 69 71 74 77 8 82 85 88 90 93 96 99 1 101 104 107 12 15 18 20 23 26 29 31 34 37 4 42 45 48 50 53 56 59 61 64 67 7 72 75 78 80 83 86 89 91 94 97 10 102 105 108 13 16 19 21 24 27 3 32 35 38 40 43 46 49 51 54 57 6 62 65 68 70 73 76 79 81 84 87 9 92 95 98

Closed 100 connections to redis with redisFree() in another thread. You'll see the file descriptor are close client-side: [root@kushan]# ls /proc/32289/fd 0 1 100 101 102 103 104 105 106 107 108 2 3 4 5 6 7 8 99

But Redis still reports 100 clients connected: 7660:M 06 Jun 2019 17:51:12.367 - DB 0: 1 keys (0 volatile) in 4 slots HT. 7660:M 06 Jun 2019 17:51:12.367 - 100 clients connected (0 replicas), 3566720 bytes in use

Because the fd are still open on its side (7660 is our redis pid) [root@kushan]# ls /proc/7660/fd 0 100 103 106 109 111 13 16 19 21 24 27 3 32 35 38 40 43 46 49 51 54 57 6 62 65 68 70 73 76 79 81 84 87 9 92 95 98 1 101 104 107 11 112 14 17 2 22 25 28 30 33 36 39 41 44 47 5 52 55 58 60 63 66 69 71 74 77 8 82 85 88 90 93 96 99 10 102 105 108 110 12 15 18 20 23 26 29 31 34 37 4 42 45 48 50 53 56 59 61 64 67 7 72 75 78 80 83 86 89 91 94 97

But: If we manually close the socket shutdown()'ing it first (clear the error before, using getsockopt) getsockopt(fd, SOL_SOCKET, SO_ERROR, (char *)&err, &len); shutdown(fd, SHUT_RDWR) close(fd)

You'll see that the socket is being closed successfully also on redis side. So I propose adding the shutdown to the redisNetClose() function.

joppino avatar Jun 06 '19 16:06 joppino