"Paravoid" posted a comment on MediaWiki.r115067. URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/115067#c32747
Commit summary for MediaWiki.r115067: Fix a segfault when more than 2 udplog listeners are configured. Paravoid's comment: Apparently, nginx has its own methods for allocating and deallocating memory using a notion of "memory pools", to avoid memory fragmentation. There is a family of routines that deal with allocating things on those pools, and a subset of those deal with something called "nginx arrays". Basically, you can create an nginx array by using ngx_array_create() and an *initial* size of the array that you want to create. You then get elements off this array by calling ngx_array_push(), and if the array is full (because you surpassed the initial size of it), it will allocate space and reallocate the *whole array* there, as to keep its memory contiguous. This has the interesting side-effect that you should never *ever* store the pointers that ngx_array_push() passes you, because they are ephemeral and may change if your array gets full/reallocated in the future. (unfortunately, documentation for the internals is very scarce... :( ). In our case, our custom udplog module kept an array of endpoints and created it with an initial size of two — something common in the nginx codebase and sensible for our case. It then added endpoints by "pushing" from the array but made the crucial mistake of storing this pointer in the "log" array (the array of structures describing what to log and where). When Peter added a third udplog socket into the configuration, the array of endpoints was full, and hence nginx reallocated it into a whole different address space. The initialization (as in, connecting to UDP sockets and whatnot) of endpoints was using the reallocated space, and saved information there, such as the sockaddr structure & socket fd. Hence, nginx started properly and spawned worker processes and threads. Then, connections were attempted and the log handler was called back; it tried to connect to its endpoint, which was referencing the *old* endpoint, before the reallocation and, more importantly, before the initalization. Hence, when tried to dereference the udp_connection pointer, which was NULL, it segfaulted and brought the entire worker process with it (which was then respawned by the master process to die again with the next request). Now, I fixed this by making separate plain memory allocations for the endpoints, using ngx_palloc() (think malloc but with pools) that are static (as in, won't be reallocated). I kept the endpoints array, so that enumeration during init is still possible, but made it contain pointers to the pointers of these allocations. For bonus points, I added a safe-guard to the handler that was dereferencing the NULL pointer as to fail if something similar happens again (shouldn't) and added comments here and there that attempt to explain the situation :-) _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
