> >I've been doing some debugging of the crashes when running PHP under > >the ISAPI sapi. Is anybody else currently looking at this? If you > >are let's talk - here are my notes so far. > > First off, can you try using > http://www.php.net/~zeev/php-4.2.3RC1.tar.gz > and see if you experience the same problems? I've made some > thread-safety related fixes there, even though none should lead > to a hard crash...
Zeev, I believe that I may have tracked down a major ISAPI filter thread-safety hole. If I understand what zend_startup() is doing correctly, it assumes that the thread that it runs under will not also be used to serve HTTP requests. I base that on the "#ifdef ZTS" section in the middle that takes the compiler_globals that has been allocated for that thread, destorys it, and replaces it with pointers to the GLOBAL_FUNCTION_TABLE and GLOBAL_CLASS_TABLE instead. I believe that the compiler_globals for this thread must thus be the "parent" of all the child threads that the web server will create (and compiler_globals_ctor() appears to bear that out). Under an ISAPI application, however, the thread that starts the DLL is also used as the first thread to serve HTTP requests through HttpExtensionProc. As PHP currently behaves, this initial thread will use the global compiler_globals as it's own personal copy (as that is the state that zend_startup() left the thread in). If another thread starts as this thread is processing, it copies the GLOBAL_FUNCTION/CLASS_TABLE assuming they are thread safe, but in reality they contain the local values of the first HTTP processing thread. The local functions and classes of the first thread leak into the second and then it's all downhill from there. I believe that the solution to this problem is to start a new thread for zend_startup() when it is called to initialize the ISAPI module. This way the globals have their own private thread and the initial thread can be safely initialized as a "child" of those global values. I've thrown together a quick hack to demonstrate this - it's not the best threaded code ever, but it has allowed me to test out my ideas. Since creating and destroying threads in DllMain is quite tricky, I've moved all the startup and shutdown code into the ISAPI GetExtensionVersion and TerminateExtension functions to make things easier. On startup GetExtensionVersion starts a new thread with php_isapi_startup_global_thread(). This thread calls the module-wide startup functions and then enters a loop waiting for ISAPI termination. When IIS calls TerminateExtension this thread is signaled to fall into the module termination code. I have never been able to get the ISAPI module to stay up for more than a few minutes when I tested previously, but I've been hammering on this code for the last hour and so far it has been rock solid. Please take a look at what I've done here and see if it makes sense. I'm trying not to get my hopes up, but what I see so far looks very encouraging. Michael Sisolak [EMAIL PROTECTED] __________________________________________________ Do You Yahoo!? Yahoo! Finance - Get real-time stock quotes http://finance.yahoo.com __________________________________________________ Do You Yahoo!? Yahoo! Finance - Get real-time stock quotes http://finance.yahoo.com
--- php-4.2.1/sapi/isapi/php4isapi.def.orig Tue Aug 27 02:36:14 2002 +++ php-4.2.1/sapi/isapi/php4isapi.def Tue Aug 27 02:36:20 2002 @@ -3,3 +3,4 @@ GetFilterVersion HttpExtensionProc GetExtensionVersion +TerminateExtension --- php-4.2.1/sapi/isapi/php4isapi.c.orig Thu Apr 18 18:52:20 2002 +++ php-4.2.1/sapi/isapi/php4isapi.c Tue Aug 27 01:54:51 2002 @@ -63,6 +63,9 @@ static zend_bool bFilterLoaded=0; static zend_bool bTerminateThreadsOnError=0; +static HANDLE pGlobalThread; +static zend_bool bGlobalThreadReady=0; + static char *isapi_special_server_variable_names[] = { "ALL_HTTP", "HTTPS", @@ -682,6 +685,32 @@ } +void php_isapi_startup_global_thread(void *unused) +{ + /* start the global thread (TSRM and global values) */ +#ifdef WITH_ZEUS + tsrm_startup(128, 1, TSRM_ERROR_LEVEL_CORE, "TSRM.log"); +#else + tsrm_startup(128, 1, TSRM_ERROR_LEVEL_CORE, "C:\\TSRM.log"); +#endif + sapi_startup(&isapi_sapi_module); + if (isapi_sapi_module.startup) { + isapi_sapi_module.startup(&sapi_module); + } + + /* signal that we're ready to begin processing requests and wait for +termination */ + bGlobalThreadReady = 1; + while (bGlobalThreadReady) + Sleep(1000); + + /* shutdown the global thread (signaled from TerminateExtension) */ + if (isapi_sapi_module.shutdown) { + isapi_sapi_module.shutdown(&sapi_module); + } + tsrm_shutdown(); +} + + BOOL WINAPI GetExtensionVersion(HSE_VERSION_INFO *pVer) { pVer->dwExtensionVersion = HSE_VERSION; @@ -690,10 +719,27 @@ #else lstrcpyn(pVer->lpszExtensionDesc, isapi_sapi_module.name, HSE_MAX_EXT_DLL_NAME_LEN); #endif + + /* start the thread to hold the global values and wait for it to initialize */ + pGlobalThread = (HANDLE)_beginthread(php_isapi_startup_global_thread, 0, NULL); + while (!bGlobalThreadReady) + Sleep(100); + + return TRUE; +} + + +BOOL WINAPI TerminateExtension(DWORD dwFlags) +{ + /* signal global thread to end and wait for it to finish */ + bGlobalThreadReady = 0; + WaitForSingleObject(pGlobalThread, INFINITE); + return TRUE; } static void my_endthread() { #ifdef PHP_WIN32 @@ -828,26 +874,14 @@ { switch (fdwReason) { case DLL_PROCESS_ATTACH: -#ifdef WITH_ZEUS - tsrm_startup(128, 1, TSRM_ERROR_LEVEL_CORE, "TSRM.log"); -#else - tsrm_startup(128, 1, TSRM_ERROR_LEVEL_CORE, "C:\\TSRM.log"); -#endif - sapi_startup(&isapi_sapi_module); - if (isapi_sapi_module.startup) { - isapi_sapi_module.startup(&sapi_module); - } break; case DLL_THREAD_ATTACH: break; case DLL_THREAD_DETACH: - ts_free_thread(); + if (bGlobalThreadReady) + ts_free_thread(); break; case DLL_PROCESS_DETACH: - if (isapi_sapi_module.shutdown) { - isapi_sapi_module.shutdown(&sapi_module); - } - tsrm_shutdown(); break; } return TRUE;
-- PHP Development Mailing List <http://www.php.net/> To unsubscribe, visit: http://www.php.net/unsub.php