Then in the old code there's also a race around the allocation of the last context, which may result in one extra completion context. It would go as follows:Finally spending some time reviewing this patch. It has a problem. Igor Nazarenko wrote:+ /* We failed to grab a context off the queue, consider allocating aThe server can have multiple accept threads (one for each Listen directive), thus mpm_get_completion_context can have multiple threads in it at once. num_completion_contexts needs protecting. I basically like your patch, but I want to spend some time thinking through potential timing windows before committing it.
+ * new one out of the child pool. There may be up to ap_threads_per_child
+ * contexts in the system at once.
+ * Note: this function assumes that only one thread may call it,
+ * so the access to num_completion_contexts variable does not
+ * have to be protected.
+ */
thread 1 checks if it can allocate another context, sees counter == max-1
thread 2 checks the same condition, sees counter == max-1
thread 1 increments the counter
thread 2 increments the counter
This will cause the server to "lose" one client connection.
The easiest way to fix it without introducing extra synchronization overhead on the main codepath is to pre-allocate all contexts during the child initialization. Should I implement that and re-submit the patch?
Igor
_________________________________________________________________
MSN 8 with e-mail virus protection service: 2 months FREE* http://join.msn.com/?page=features/virus
