Skip to content

Commit 92a2633

Browse files
committed
bugfix: fixed a race condition in the new conn queuing mechanism.
This issue appeared in our EC2 test cluster, which compiles Nginx with `-DNGX_LUA_USE_ASSERT` and `-DNGX_LUA_ABORT_AT_PANIC`. The lua-resty-redis test: === TEST 1: github issue openresty#108: ngx.locaiton.capture + redis.set_keepalive in t/bugs.t [1] would produce core dumps in the check leak testing mode. The backtrace for these core dumps was: #0 0x00007fd417bee277 in raise () from /lib64/libc.so.6 openresty#1 0x00007fd417bef968 in abort () from /lib64/libc.so.6 openresty#2 0x00007fd417be7096 in __assert_fail_base () from /lib64/libc.so.6 openresty#3 0x00007fd417be7142 in __assert_fail () from /lib64/libc.so.6 openresty#4 0x000000000050d227 in ngx_http_lua_socket_tcp_resume_conn_op (spool=c/ngx_http_lua_socket_tcp.c:3963 openresty#5 0x000000000050e51a in ngx_http_lua_socket_tcp_finalize (r=r@entry=0x5628) at ../../src/ngx_http_lua_socket_tcp.c:4195 openresty#6 0x000000000050e570 in ngx_http_lua_socket_tcp_cleanup (data=0x7fd419p_lua_socket_tcp.c:3755 openresty#7 0x0000000000463aa5 in ngx_http_free_request (r=r@entry=0xbfaec0, rc=http_request.c:3508 ... Which was caused by the following assertion in ngx_http_lua_socket_tcp.c with `NGX_DEBUG`: #if (NGX_DEBUG) ngx_http_lua_assert(spool->connections >= 0); #else Thanks to Mozilla's rr, a recorded session showed that `spool->connections` was `-1`. Here is a reproducible test case: local sock1 = ngx.socket.tcp() local sock2 = ngx.socket.tcp() sock1:connect() sock2:connect() sock1:setkeepalive() -- pool created, connections: 1 sock2:setkeepalive() -- connections: 1 sock1:connect() -- connections: 1 sock2:connect() -- connections: 1 sock1:close() -- connections: 0 sock2:close() -- connections: -1 -- ngx_http_lua_socket_tcp_resume_conn_op gets called, assertion fails In order to avoid this race condition, we must determine whether the socket pool exists or not, not from the `ngx_http_lua_socket_tcp_upstream` struct, but from the Lua Registry. This way, when thread 2's socket enters the keepalive state, it will respect the previous call to `ngx_http_lua_socket_free_pool` (which unset the pool from the registry). [1]: https://github.com/openresty/lua-resty-redis/blob/master/t/bugs.t
1 parent fd90f4e commit 92a2633

File tree

1 file changed

+14
-0
lines changed

1 file changed

+14
-0
lines changed

src/ngx_http_lua_socket_tcp.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4132,6 +4132,7 @@ static void
41324132
ngx_http_lua_socket_tcp_finalize(ngx_http_request_t *r,
41334133
ngx_http_lua_socket_tcp_upstream_t *u)
41344134
{
4135+
lua_State *L;
41354136
ngx_connection_t *c;
41364137
ngx_http_lua_socket_pool_t *spool;
41374138

@@ -4185,6 +4186,19 @@ ngx_http_lua_socket_tcp_finalize(ngx_http_request_t *r,
41854186
return;
41864187
}
41874188

4189+
L = spool->lua_vm;
4190+
4191+
lua_pushlightuserdata(L, ngx_http_lua_lightudata_mask(socket_pool_key));
4192+
lua_rawget(L, LUA_REGISTRYINDEX);
4193+
lua_pushstring(L, (char *) spool->key);
4194+
lua_rawget(L, -2);
4195+
spool = lua_touserdata(L, -1);
4196+
lua_pop(L, 1);
4197+
4198+
if (spool == NULL) {
4199+
return;
4200+
}
4201+
41884202
spool->connections--;
41894203

41904204
if (spool->connections == 0) {

0 commit comments

Comments
 (0)