Old nginx worker got into dead loop when reload

[jinglong]

Old nginx worker got into dead loop when reload. The version of lua-module is 0.9.6, version of nginx is 1.2.9. The backtrace is:
#0 ngx_http_lua_abort_pending_timers (ev=) at mod_lua/src/ngx_http_lua_timer.c:495
#1 0x00000000004340da in ngx_worker_process_cycle (cycle=0x443ded0, data=) at src/os/unix/ngx_process_cycle.c:839

...
This problem happenned when travelling the rb-tree to find the peeding timers. The parent node of the root of the rb-tree in Nginx could be Non-NULL and Non-sentinel, for nginx don't guarantee that it shold be NULL or sentinel.

(gdb) p *ngx_event_timer_rbtree.root
$6 = {key = 1398068596611, left = 0x4440528, right = 0x4443078, parent = 0x4440528, color = 0 '\000', data = 0 '\000'}

As above, the parent of root is equal to the left child of root. So this causes ngx_http_lua_abort_pending_timers went into dead loop.

I think it should be easier to travelling the rbtree using a (extra) stack, for there is no flag or something we can use in a rb-tree node.

[agentzh]

@jinglong Thanks for the report! I think a better fix is to explicitly test if the current node is the root (just as how the NGINX core does itself). I don't like the overhead of an extra stack. Will you submit a patch for this?

[agentzh]

@jinglong What do you think of the following patch?

diff --git a/src/ngx_http_lua_timer.c b/src/ngx_http_lua_timer.c
index 120c528..3fde3ef 100644
--- a/src/ngx_http_lua_timer.c
+++ b/src/ngx_http_lua_timer.c
@@ -419,7 +419,7 @@ ngx_http_lua_abort_pending_timers(ngx_event_t *ev)
     ngx_int_t                    i, n;
     ngx_event_t                **events;
     ngx_connection_t            *c, *saved_c = NULL;
-    ngx_rbtree_node_t           *cur, *prev, *next, *sentinel;
+    ngx_rbtree_node_t           *root, *cur, *prev, *next, *sentinel;
     ngx_http_lua_timer_ctx_t    *tctx;
     ngx_http_lua_main_conf_t    *lmcf;

@@ -462,7 +462,12 @@ ngx_http_lua_abort_pending_timers(ngx_event_t *ev)

     sentinel = ngx_event_timer_rbtree.sentinel;

-    cur = ngx_event_timer_rbtree.root;
+    root = ngx_event_timer_rbtree.root;
+
+    /* XXX nginx does not guarentee the parent of root is meaningful */
+    root->parent = NULL;
+
+    cur = root;
     prev = cur->parent;

     events = ngx_pcalloc(ngx_cycle->pool,

[agentzh]

@jinglong I've just committed a safer version of my previous patch to git master, which restores the old value of the root's parent. It is still a bit hacky but it simplies our rbtree traversal a lot :)

Will you try it out on your side? I still do not have enough luck to reproduce the infinite loop on my side. Thanks!

[agentzh]

Consider it resolved.