A possible bug in ngx_rbtree

[Maxim Dounin]

Hello!

On Tue, Oct 02, 2012 at 11:08:27AM -0400, YongFeng Wu wrote:

> 
> Hi Maxim,
> 
> Thank you so much for looking into it:
> 
> Following is the nginx -V output:
> 
> nginx version: nginx/1.2.3
> built by gcc 4.2.1 20070719  [FreeBSD]
> configure arguments: --with-pcre --with-debug --with-http_geoip_module
> --with-http_stub_status_module

Is geoip module actually used?  Unfortunately MaxMind's GeoIP 
library it uses is known to do bad things if used with corrupted 
database file, this may be a culprit.

It might be also good idea to make sure hardware problems are 
ruled out.  From your initial message I conclude you've only seen 
this only once on a single server, is it correct?

> Following is the content for cache, cache->rbtree, root and sentinel:
> 
> (gdb) p cache
> $5 = (ngx_open_file_cache_t *) 0x8012495f8
> (gdb) p *cache
> $6 = {rbtree = {root = 0x803377580, sentinel = 0x801249610, insert =
> 0x423ba0 <ngx_open_file_cache_rbtree_insert_value>}, sentinel = {
>     key = 0, left = 0x0, right = 0x0, parent = 0x8044d4100, color = 0 '\0',
> data = 0 '\0'}, expire_queue = {prev = 0x8044d4ba8,
>     next = 0x803376228}, current = 999, max = 1000, inactive = 60}
> (gdb) p cache->rbtree
> $7 = {root = 0x803377580, sentinel = 0x801249610, insert = 0x423ba0
> <ngx_open_file_cache_rbtree_insert_value>}
> (gdb) p *cache->rbtree->root
> $8 = {key = 2661524630, left = 0x801230100, right = 0x8032b4100, parent =
> 0x0, color = 0 '\0', data = 46 '.'}

[...]

Looks fine, i.e. nothing suspicious here.

> One thing weird is that the sentinel->parent is not null but set to a node.
> Could this cause some problems? sentinel->parent could be set in code like
> (function ngx_rbtree_delete, line 209 in ngx_rbtree.c, version 1.2.3):
> 
>     if (subst == node) {
> 
>         temp->parent = subst->parent;
> 
>     } else {

This code is not exactly right (and catched my eye during 
re-looking though the code, too) and we might want to change it to 
do nothing if temp == sentinel, but it should be completely 
harmless.

-- 
Maxim Dounin
http://nginx.com/support.html