php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #59950 Race condition on shutdown (or graceful restart) crashes Apache
Submitted: 2011-09-13 01:24 UTC Modified: 2011-09-15 05:03 UTC
From: Brian dot White at foxfire74 dot com Assigned: pajoye (profile)
Status: Closed Package: APC (PECL)
PHP Version: 5.3.6 OS: Windows
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: Brian dot White at foxfire74 dot com
New email:
PHP Version: OS:

 

 [2011-09-13 01:24 UTC] Brian dot White at foxfire74 dot com 
Description:
------------
APC fails on restart when PHP is run as an Apache module on Windows because of a race condition on shutdown between the parent and child processes.  Both processes call DESTROY_LOCK for the same object which in is a wrapper for RtlDeleteResource when windows kernel locking is enabled.  The first one succeeds, the second one fails since WINAPI function RtlDeleteResource MUST be called with a valid lock.  This is only an issue if the new OS kernel lock type is enabled.  A patch which fixes this issue is attached.  The patch  makes apc_windows_srwlock_kernel behave similar to the other lock types in that they will succeed if the lock structure has already been released/cleared.

Reproduce code:
---------------
--- W:/TEMP/APC-3.1.9/apc_windows_srwlock_kernel.c	Sat May 14 18:14:56 2011
+++ W:/php-sdk/php53dev/vc9/x86/php-5.3.8/ext/apc/apc_windows_srwlock_kernel.c	Mon Sep 12 22:00:00 2011
@@ -81,7 +81,14 @@
 
 void apc_windows_cs_destroy(apc_windows_cs_rwlock_t *lock)
 {
-    pRtlDeleteResource(lock);
+    __try
+    {
+        pRtlDeleteResource(lock);
+    }
+    __except(GetExceptionCode() == EXCEPTION_ACCESS_VIOLATION ? EXCEPTION_EXECUTE_HANDLER : EXCEPTION_CONTINUE_SEARCH)
+    {
+        /* Ignore exception (resource was freed during shutdown of another thread) */
+    }
     FreeLibrary(ntdll);
     return;
 }

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2011-09-14 05:22 UTC] at php dot net 
Can you try using the patch at 
http://pecl.php.net/bugs/bug.php?id=23822 pls? The call of 
this function should be thread safe in the 1st place.
 [2011-09-14 08:24 UTC] Brian dot White at foxfire74 dot com 
I submitted that patch for a typo too since and that patch is proper/correct but doesn't completely fix the problem.  Before that patch, APC crashed Apache on the first call to DESTROY_LOCK (line 890 in apc_main.c) when it tries to destroy a non-blocking lock that was never created.  For my configuration, I have three locks in the OS; one for the op code cache, one for the user cache and one for the SMA.  When apc shuts down apc_cache_destroy() is called by the child process which releases the OS resources first for the op_code cache lock then for the user cache.  Then the child does shared memory lock clean up.  When the child exits, there are no more locks in the system; but the parent tries to destroy the same three locks as well which also causes the problem.  My first attempt to resolve the solution involved tracking the PIDs which created the cache locks and only allowing the creating process to destroy the lock. That got things further into apc_module_shutdown but I discovered that apc_sma_cleanup is also called twice.

This is a beginning of time issue for APC as a module on Windows; but didn't manifest itself until now because the other lock types don't fail when trying to release the resources for a lock which have already been previously released.  If you comment out the actual call to RtlDeleteResource and replace it with a debugging statement you will see that it is called six times when it only should be called three times.  A call to RtlDumpResource in the same location may also be informative.  Strictly speaking, there are "extra" calls to RtlInitializeResource; but they are harmless since, at initialization, the lock struct points to a valid memory location.   (So, in fact, there may be a very small leak of OS resources.)  If you dump the PID and the lock address somewhere in apc_windows_cs_create you can see what I am talking about.

Also I couldn't check for lock == 0 in apc_windows_cs_destroy because the lock is destroyed in a critical section in the OS after both processes have BEGUN the call to RtlDeleteResource.  So I couldn't find a good way to trap the race and pick a winner in the APC code which is why I resorted to __try/__except.
 [2011-09-14 08:28 UTC] at php dot net 
ok, I was wondering because this function is called on 
module_shutdown and module_shutdown should not be called many 
times.

In any case, this fix won't hurt. Let me apply it.
 [2011-09-14 08:42 UTC] Brian dot White at foxfire74 dot com 
Thanks for being so responsive on this.  There are guards in apc_module_shutdown to prevent it from being called more than once; but it's an odd race condition for APC on windows when using mod_php.  These days, I think PHP is more commonly used as a fast CGI module which doesn't have the issue.

BTW: Thanks for implementing the kernel locks.  I have observed a definite performance increase.
 [2011-09-15 05:03 UTC] at php dot net 
This bug has been fixed in SVN.

In case this was a documentation problem, the fix will show up at the
end of next Sunday (CET) on pecl.php.net.

In case this was a pecl.php.net website problem, the change will show
up on the website in short time.
 
Thank you for the report, and for helping us make PECL better.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Wed May 15 18:01:34 2024 UTC