A patch to fix one sort of mod_perl crash I've been experiencing.

Reading MP_TRACE messages I noticed that for every modperl_interp_select() call during a request there were two modperl_interp_unselect() calls, one bracketing the _select() call and one from the registered cleanup when the pool was destroyed. The attached patch stopped those crashes. I would call my fix VERY tentative  for various reasons including my noobiness and I've added comments near the active ingredient in modperl_interp.c. Inactive ingredients include diffs in the patch which are my adding trace info.

Now I'm chasing a bug which causes a much less frequent crash.

This patch is against the version of mod_perl I checked out some months ago: 1566538 which is in httpd24threading, but having checked the changes to that branch since then I didn't find an obvious conflict.

This is my httpd:

Server version: Apache/2.2.26 (Win32)
Server built:   Mar 17 2014 09:15:35
Server's Module Magic Number: 20051115:33
Server loaded:  APR 1.4.8, APR-Util 1.5.2
Compiled using: APR 1.4.8, APR-Util 1.5.2
Architecture:   32-bit
Server MPM:     WinNT
  threaded:     yes (fixed thread count)
    forked:     no
Server compiled with....
 -D APACHE_MPM_DIR="server/mpm/winnt"
 -D APR_HAS_SENDFILE
 -D APR_HAS_MMAP
 -D APR_HAS_OTHER_CHILD
 -D AP_HAVE_RELIABLE_PIPED_LOGS
 -D DYNAMIC_MODULE_LIMIT=128
 -D HTTPD_ROOT="/apache"
 -D DEFAULT_SCOREBOARD="logs/apache_runtime_status"
 -D DEFAULT_ERRORLOG="logs/error.log"
 -D AP_TYPES_CONFIG_FILE="conf/mime.types"
 -D SERVER_CONFIG_FILE="conf/httpd.conf"

And my Perl:

Summary of my perl5 (revision 5 version 18 subversion 2) configuration:

  Platform:
    osname=MSWin32, osvers=6.1, archname=MSWin32-x86-multi-thread
    uname=''
    config_args='undef'
    hint=recommended, useposix=true, d_sigaction=undef
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=undef, use64bitall=undef, uselongdouble=undef
    usemymalloc=y, bincompat5005=undef
  Compiler:
    cc='cl', ccflags ='-nologo -GF -W3 -O1 -MD -Zi -DNDEBUG -DWIN32 -D_CONSOLE -
DNO_STRICT -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE  -DPERL_TEXTMO
DE_SCRIPTS -DPERL_IMPLICIT_CONTEXT -DUSE_PERLIO',
    optimize='-O1 -MD -Zi -DNDEBUG',
    cppflags='-DWIN32'
    ccversion='15.00.30729.01', gccversion='', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=8
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='__int64', lseeksi
ze=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='link', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf  -libpath:"c:
\perl\5.18.2\lib\MSWin32-x86-multi-thread\CORE"  -machine:x86 "/manifestdependen
cy:type='Win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' proce
ssorArchitecture='*' publicKeyToken='6595b64144ccf1df' language='*'"'
    libpth=\lib
    libs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib  comdlg32.l
ib advapi32.lib shell32.lib ole32.lib oleaut32.lib  netapi32.lib uuid.lib ws2_32
.lib mpr.lib winmm.lib  version.lib odbc32.lib odbccp32.lib comctl32.lib msvcrt.
lib
    perllibs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib  comdlg
32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib  netapi32.lib uuid.lib ws
2_32.lib mpr.lib winmm.lib  version.lib odbc32.lib odbccp32.lib comctl32.lib msv
crt.lib
    libc=msvcrt.lib, so=dll, useshrplib=true, libperl=perl518.lib
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug -opt:ref,icf  -
libpath:"c:\perl\5.18.2\lib\MSWin32-x86-multi-thread\CORE"  -machine:x86 "/manif
estdependency:type='Win32' name='Microsoft.Windows.Common-Controls' version='6.0
.0.0' processorArchitecture='*' publicKeyToken='6595b64144ccf1df' language='*'"'



Characteristics of this binary (from libperl):
  Compile-time options: HAS_TIMES HAVE_INTERP_INTERN MULTIPLICITY MYMALLOC
                        PERLIO_LAYERS PERL_DONT_CREATE_GVSV
                        PERL_HASH_FUNC_ONE_AT_A_TIME_HARD
                        PERL_IMPLICIT_CONTEXT PERL_MALLOC_WRAP
                        PERL_PRESERVE_IVUV PERL_SAWAMPERSAND USE_ITHREADS
                        USE_LARGE_FILES USE_LOCALE USE_LOCALE_COLLATE
                        USE_LOCALE_CTYPE USE_LOCALE_NUMERIC USE_PERLIO
                        USE_PERL_ATOF
  Built under MSWin32
  Compiled at Mar 12 2014 12:48:53
  %ENV:
    PERL5LIB="c:/cygwin/home/085598/lib;c:/cygwin/home/085598/Perl"
    PERLDOC_PAGER="less -ir"
  @INC:
    c:/cygwin/home/085598/lib
    c:/cygwin/home/085598/Perl
    C:/perl/site/5.18.2/lib/MSWin32-x86-multi-thread
    C:/perl/site/5.18.2/lib
    C:/perl/5.18.2/lib/MSWin32-x86-multi-thread
    C:/perl/5.18.2/lib
    .

Both build with VC 2008

Richard Kandarian
http://www.lanl.gov/cgi-bin/fonelink.pl/085598
My node in the Web: http://www.kandarian.com

Index: src/modules/perl/mod_perl.c
===================================================================
--- src/modules/perl/mod_perl.c	(revision 1574926)
+++ src/modules/perl/mod_perl.c	(working copy)
@@ -56,7 +56,8 @@
 static apr_status_t modperl_shutdown(void *data)
 {
     modperl_cleanup_data_t *cdata = (modperl_cleanup_data_t *)data;
-    PerlInterpreter *perl = (PerlInterpreter *)cdata->data;
+    dTHXa(cdata->data)
+    tTHX perl = aTHX;
     void **handles;
 
     handles = modperl_xs_dl_handles_get(aTHX);
Index: src/modules/perl/modperl_config.c
===================================================================
--- src/modules/perl/modperl_config.c	(revision 1574926)
+++ src/modules/perl/modperl_config.c	(working copy)
@@ -143,7 +143,7 @@
     modperl_config_req_t *rcfg =
         (modperl_config_req_t *)apr_pcalloc(r->pool, sizeof(*rcfg));
 
-    MP_TRACE_d(MP_FUNC, "0x%lx", (unsigned long)rcfg);
+    MP_TRACE_d(MP_FUNC, "new request, rec: %pp rcfg: %pp", r, rcfg);
 
     return rcfg;
 }
Index: src/modules/perl/modperl_interp.c
===================================================================
--- src/modules/perl/modperl_interp.c	(revision 1574926)
+++ src/modules/perl/modperl_interp.c	(working copy)
@@ -274,17 +274,22 @@
     modperl_interp_pool_t *mip = interp->mip;
 
     MP_ASSERT(interp && MpInterpIN_USE(interp));
-    MP_TRACE_i(MP_FUNC, "unselect(interp=%pp): refcnt=%d",
-               interp, interp->refcnt);
+    MP_TRACE_i(MP_FUNC, "unselect(interp=%pp): refcnt=%d in use: %d",
+               interp, interp->refcnt, MpInterpIN_USE(interp));
 
-    if (interp->refcnt > 1) {
+    if (interp->refcnt > 1) { /* Why not abort if refcnt <= 0? */
         --interp->refcnt;
         MP_TRACE_i(MP_FUNC, "interp=0x%lx, refcnt=%d -- interp still in use",
                    (unsigned long)interp, interp->refcnt);
         return APR_SUCCESS;
     }
+    if(!MpInterpIN_USE(interp)){
+        MP_TRACE_i(MP_FUNC, "interp=0x%lx, refcnt=%d -- interp not in use, already unselected?",
+                   (unsigned long)interp, interp->refcnt);
+        return APR_SUCCESS;
+    }
 
-    MpInterpIN_USE_Off(interp);
+    MpInterpIN_USE_Off(interp); /* Why not zero refcnt? */
 
     modperl_thx_interp_set(interp->perl, NULL);
 #ifdef MP_DEBUG
@@ -438,8 +443,8 @@
         ccfg->interp->refcnt++;
 
         MP_TRACE_i(MP_FUNC,
-                   "found interp 0x%lx in con config, refcnt incremented to %d",
-                   (unsigned long)ccfg->interp, ccfg->interp->refcnt);
+	 "req: %pp, found interp 0x%lx in con config, refcnt incremented to %d",
+	   r, (unsigned long)ccfg->interp, ccfg->interp->refcnt);
         /* set context (THX) for this thread */
         PERL_SET_CONTEXT(ccfg->interp->perl);
         /* modperl_thx_interp_set() is not called here because the interp
@@ -449,9 +454,11 @@
     }
 
     MP_TRACE_i(MP_FUNC,
-               "fetching interp for %s:%d", s->server_hostname, s->port);
+               "req: %pp, fetching interp for %s:%d, ccfg: %pp",
+	       r, s->server_hostname, s->port, ccfg);
     interp = modperl_interp_get(s);
-    MP_TRACE_i(MP_FUNC, "  --> got %pp (perl=%pp)", interp, interp->perl);
+    MP_TRACE_i(MP_FUNC,
+      "req: %pp,   --> got %pp (perl=%pp)", r, interp, interp->perl);
     ++interp->num_requests; /* should only get here once per request */
     interp->refcnt = 0;
 
@@ -466,8 +473,8 @@
     interp->ccfg = ccfg;
 
     MP_TRACE_i(MP_FUNC,
-               "pulled interp %pp (perl=%pp) from mip, num_requests is %d",
-               interp, interp->perl, interp->num_requests);
+               "req: %pp, pulled interp %pp (perl=%pp) from mip, num_requests is %d, ccfg: %pp",
+               r, interp, interp->perl, interp->num_requests, ccfg);
 
     /*
      * if a per-dir PerlInterpScope is specified, use it.