> >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

Reply via email to