You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by mt...@apache.org on 2007/03/15 12:11:22 UTC

svn commit: r518581 - /tomcat/connectors/trunk/jk/native/common/jk_shm.c

Author: mturk
Date: Thu Mar 15 04:11:20 2007
New Revision: 518581

URL: http://svn.apache.org/viewvc?view=rev&rev=518581
Log:
Fix shared memory lock by using temporary file instead
adding extension to shared memory file. This allows to
have higher security for shared memory

Modified:
    tomcat/connectors/trunk/jk/native/common/jk_shm.c

Modified: tomcat/connectors/trunk/jk/native/common/jk_shm.c
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_shm.c?view=diff&rev=518581&r1=518580&r2=518581
==============================================================================
--- tomcat/connectors/trunk/jk/native/common/jk_shm.c (original)
+++ tomcat/connectors/trunk/jk/native/common/jk_shm.c Thu Mar 15 04:11:20 2007
@@ -59,6 +59,7 @@
 {
     size_t     size;
     const char *filename;
+    const char *lockname;
     int        fd;
     int        fd_lock;
     int        attached;
@@ -69,7 +70,7 @@
 typedef struct jk_shm jk_shm_t;
 
 static const char shm_signature[] = { JK_SHM_MAGIC };
-static jk_shm_t jk_shmem = { 0, NULL, -1, -1, 0, NULL};
+static jk_shm_t jk_shmem = { 0, NULL, NULL, -1, -1, 0, NULL};
 static time_t jk_workers_modified_time = 0;
 static time_t jk_workers_access_time = 0;
 #if defined (WIN32)
@@ -227,44 +228,68 @@
 #define MAP_FILE    (0)
 #endif
 
-static int do_shm_open_lock(const char *fname, int attached, jk_logger_t *l)
+static int do_shm_open_lock(int attached, jk_logger_t *l)
 {
     int rc;
-    int fd;
-    int flags = O_RDWR;
     char flkname[256];
     JK_TRACE_ENTER(l);
 
-    jk_shmem.fd_lock = -1;
-    strcpy(flkname, fname);
-    strcat(flkname, ".lock");
-    if (!attached)
-        flags |= (O_CREAT|O_TRUNC);
-    fd = open(flkname, flags, 0666);
-    if (fd == -1) {
+    if (!jk_shmem.lockname) {
+        int i;
+        jk_shmem.fd_lock = -1;
+        for (i = 0; i < 8) {
+            strcpy(flkname, "/tmp/jkshmlock.XXXXXX");
+            if (mktemp(flkname)) {
+                jk_shmem.fd_lock = open(flkname, O_RDWR|O_CREAT|O_TRUNC, 0666);
+                if (jk_shmem.fd_lock >= 0)
+                    break;
+            }
+        }
+        if (jk_shmem.fd_lock == -1) {
+            rc = errno;
+            JK_TRACE_EXIT(l);
+            return rc;
+        }
+        jk_shmem.lockname = strdup(flkname);
+        unlink(jk_shmem.lockname);
+    }
+    else if (attached) {
+        jk_shmem.fd_lock = open(jk_shmem.lockname, O_RDWR, 0666);
+        if (jk_shmem.fd_lock == -1) {
+            rc = errno;
+            JK_TRACE_EXIT(l);
+            return rc;
+        }
+        if (JK_IS_DEBUG_LEVEL(l))
+            jk_log(l, JK_LOG_DEBUG,
+                   "Duplicated shared memory lock %s", jk_shmem.lockname);
         JK_TRACE_EXIT(l);
-        return errno;
+        return 0;
+    }
+    else {
+        
+        
     }
 
     if (!attached) {
-        if (ftruncate(fd, 1)) {
+        if (ftruncate(jk_shmem.fd_lock, 1)) {
             rc = errno;
-            close(fd);
+            close(jk_shmem.fd_lock);
+            jk_shmem.fd_lock = -1;
             JK_TRACE_EXIT(l);
             return rc;
          }
     }
-    if (lseek(fd, 0, SEEK_SET) != 0) {
+    if (lseek(jk_shmem.fd_lock, 0, SEEK_SET) != 0) {
         rc = errno;
-        close(fd);
+        close(jk_shmem.fd_lock);
+        jk_shmem.fd_lock = -1;
         JK_TRACE_EXIT(l);
         return rc;
     }
-    jk_shmem.fd_lock = fd;
-
     if (JK_IS_DEBUG_LEVEL(l))
         jk_log(l, JK_LOG_DEBUG,
-               "Opened shared memory lock %s", flkname);
+               "Opened shared memory lock %s", jk_shmem.lockname);
     JK_TRACE_EXIT(l);
     return 0;
 }
@@ -274,7 +299,6 @@
 {
     int rc;
     int fd;
-    int flags = O_RDWR;
     void *base;
 
     JK_TRACE_ENTER(l);
@@ -283,12 +307,14 @@
         if (!attached)
             attached = 1;
     }
+    else if (attached) {
+        /* We should already have a header
+         * Use memory if we don't
+         */
+        JK_TRACE_EXIT(l);
+        return 0;
+    }
     jk_shmem.filename = fname;
-    if (attached)
-        jk_shmem.attached = (int)getpid();
-    else
-        jk_shmem.attached = 0;
-
     jk_shmem.size = JK_SHM_ALIGN(sizeof(jk_shm_header_t) + sz);
 
     /* Use plain memory in case there is no file name */
@@ -301,17 +327,16 @@
         return 0;
     }
 
-    if (!attached)
-        flags |= (O_CREAT|O_TRUNC);
-    fd = open(fname, flags, 0666);
-    if (fd == -1) {
-        jk_shmem.size = 0;
-        JK_TRACE_EXIT(l);
-        return errno;
-    }
-
     if (!attached) {
-        size_t size = lseek(fd, 0, SEEK_END);
+        size_t size;
+        jk_shmem.attached = 0;
+        fd = open(fname, O_RDWR|O_CREAT|O_TRUNC, 0666);
+        if (fd == -1) {
+            jk_shmem.size = 0;
+            JK_TRACE_EXIT(l);
+            return errno;
+        }
+        size = lseek(fd, 0, SEEK_END);
         if (size < jk_shmem.size) {
             size = jk_shmem.size;
             if (ftruncate(fd, jk_shmem.size)) {
@@ -325,31 +350,27 @@
                 jk_log(l, JK_LOG_DEBUG,
                        "Truncated shared memory to %u", size);
         }
-    }
-    if (lseek(fd, 0, SEEK_SET) != 0) {
-        rc = errno;
-        close(fd);
-        jk_shmem.size = 0;
-        JK_TRACE_EXIT(l);
-        return rc;
-    }
-
-    base = mmap((caddr_t)0, jk_shmem.size,
-                PROT_READ | PROT_WRITE,
-                MAP_FILE | MAP_SHARED,
-                fd, 0);
-    if (base == (caddr_t)MAP_FAILED || base == (caddr_t)0) {
-        rc = errno;
-        close(fd);
-        jk_shmem.size = 0;
-        JK_TRACE_EXIT(l);
-        return rc;
-    }
-    jk_shmem.hdr = base;
-    jk_shmem.fd  = fd;
+        if (lseek(fd, 0, SEEK_SET) != 0) {
+            rc = errno;
+            close(fd);
+            jk_shmem.size = 0;
+            JK_TRACE_EXIT(l);
+            return rc;
+        }
 
-    /* Clear shared memory */
-    if (!attached) {
+        base = mmap((caddr_t)0, jk_shmem.size,
+                    PROT_READ | PROT_WRITE,
+                    MAP_FILE | MAP_SHARED,
+                    fd, 0);
+        if (base == (caddr_t)MAP_FAILED || base == (caddr_t)0) {
+            rc = errno;
+            close(fd);
+            jk_shmem.size = 0;
+            JK_TRACE_EXIT(l);
+            return rc;
+        }
+        jk_shmem.hdr = base;
+        jk_shmem.fd  = fd;
         memset(jk_shmem.hdr, 0, jk_shmem.size);
         memcpy(jk_shmem.hdr->h.data.magic, shm_signature, JK_SHM_MAGIC_SIZ);
         jk_shmem.hdr->h.data.size = sz;
@@ -360,11 +381,12 @@
                    jk_shmem.size, jk_shmem.hdr->h.data.size, jk_shmem.hdr);
     }
     else {
-        jk_shmem.hdr->h.data.childs++;
+        unsigned int nchild = jk_shmem.hdr->h.data.childs + 1;
+        jk_shmem.attached = (int)getpid();
         if (JK_IS_DEBUG_LEVEL(l))
             jk_log(l, JK_LOG_INFO,
                    "Attached shared memory [%d] size=%u free=%u addr=%#lx",
-                   jk_shmem.hdr->h.data.childs, jk_shmem.hdr->h.data.size,
+                   nchild, jk_shmem.hdr->h.data.size,
                    jk_shmem.hdr->h.data.size - jk_shmem.hdr->h.data.pos,
                    jk_shmem.hdr);
         /*
@@ -374,20 +396,23 @@
          * if the number of workers change between
          * open and attach or between two attach operations.
          */
-        if (jk_shmem.hdr->h.data.childs > 1) {
+        if (nchild > 1) {
             if (JK_IS_DEBUG_LEVEL(l)) {
                 jk_log(l, JK_LOG_DEBUG,
                        "Reseting the shared memory for child %d",
-                       jk_shmem.hdr->h.data.childs);
+                       nchild);
             }
         }
+        jk_shmem.hdr->h.data.childs  = nchild;
         jk_shmem.hdr->h.data.pos     = 0;
         jk_shmem.hdr->h.data.workers = 0;
     }
     JK_INIT_CS(&(jk_shmem.cs), rc);
-    if ((rc = do_shm_open_lock(fname, attached, l))) {
-        munmap((void *)jk_shmem.hdr, jk_shmem.size);
-        close(jk_shmem.fd);
+    if ((rc = do_shm_open_lock(attached, l))) {
+        if (!attached) {
+            munmap((void *)jk_shmem.hdr, jk_shmem.size);
+            close(jk_shmem.fd);
+        }
         jk_shmem.hdr = NULL;
         jk_shmem.fd  = -1;
         JK_TRACE_EXIT(l);
@@ -413,27 +438,32 @@
     if (jk_shmem.hdr) {
         --jk_shmem.hdr->h.data.childs;
 
+        if (jk_shmem.fd_lock >= 0) {
+            close(jk_shmem.fd_lock);
+            jk_shmem.fd_lock = -1;
+        }
+        JK_DELETE_CS(&(jk_shmem.cs), rc);
         if (jk_shmem.attached) {
             int p = (int)getpid();
-            if (p != jk_shmem.attached) {
+            if (p == jk_shmem.attached) {
                 /* In case this is a forked child
                  * do not close the shared memory.
                  * It will be closed by the parent.
                  */
-                 return;
+                jk_shmem.size = 0;
+                jk_shmem.hdr  = NULL;
+                jk_shmem.fd   = -1;
+                return;
             }
         }
-        if (jk_shmem.fd_lock >= 0) {
-            close(jk_shmem.fd_lock);
-        }
         if (jk_shmem.fd >= 0) {
             munmap((void *)jk_shmem.hdr, jk_shmem.size);
             close(jk_shmem.fd);
+            if (jk_shmem.lockname) {
+                free(jk_shmem.lockname);
+                jk_shmem.lockname = NULL;
+            }
         }
-        jk_shmem.fd_lock = -1;
-    }
-    if (jk_shmem.size) {
-        JK_DELETE_CS(&(jk_shmem.cs), rc);
     }
     jk_shmem.size = 0;
     jk_shmem.hdr  = NULL;



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r518581 - /tomcat/connectors/trunk/jk/native/common/jk_shm.c

Posted by Mladen Turk <mt...@apache.org>.
Rainer Jung wrote:
> 
> I have the impression, that this would be nice for our shm file too.
>

Right, it can be done with ease, just like we append .lock to
JkShmFile we can append the pid

Regards,
Mladen.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r518581 - /tomcat/connectors/trunk/jk/native/common/jk_shm.c

Posted by Rainer Jung <ra...@kippdata.de>.
Of course. I want it the other way round. Adding the pid to the file 
name would prevent users from erroneously sharing the shm file via it's 
default value.

Consider users who have an apache httpd installed as a product and 2 
separate instances (conf, logs, run etc.) defined. Since httpd does not 
really have a nice way to distinguish between the product and the 
instance (like catalina_home and catalina_base), you have to decide if 
you set your ServerRoot to the product directory (and then use full path 
names for the files in the instance) or the other way round.

If a user has ServerRoot set to the product directory and explicitely 
sets the conf and logs files to the instance directory, but does not set 
an explicit JkShmFile, all instances will share the same JkShmFile in 
the ServerRoot logs, which will obviously produce *very* bad results.

Even more: if they put their JkShmFile into a shared run directory (like 
/var/apache/run) and don't communicate about not using the same name, 
the same problem will happen. For this reason most of the usual httpd 
lock/pid/status etc. files already include the parent process pid in 
their file name, so that no clash will happen. Some of the files, which 
did not include the pid for httpd 2.0, do for 2.2.

I have the impression, that this would be nice for our shm file too.

Regards,

Rainer

Mladen Turk wrote:
> Rainer Jung wrote:
>> Hi Mladen,
>>
>> I didn't really read this commit, but it's Log entry reminds me of the 
>> following:
>>
> 
> This is for shm lock, not for the shared memory, so the name is
> irrelevant, and will be destroyed on exit.
> 
> Also, we store workers in shared memory, so any attempt to
> have a shared memory between instances is tricky, because
> it can lead to a total mess if the configuration is not
> exactly the same (even ordering matters).
> 
> Regards,
> Mladen.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r518581 - /tomcat/connectors/trunk/jk/native/common/jk_shm.c

Posted by Mladen Turk <mt...@apache.org>.
Jim Jagielski wrote:
> 
> On Mar 15, 2007, at 9:36 AM, Mladen Turk wrote:
> 
>> Jim Jagielski wrote:
>>> Do we *really* want 0666 on these things? Wouldn't 0644 be better?
>>
>> Of course. I just made that compile time enabled for faulty kernels.
>> I'll check if the 0644 works for flock.
>>
> 
> It should... these are opened early enough and since
> the fd's are shared, the subprocesses/threads should
> still have access. Under Apache 1.3 we even have
> them 0600 :)
> 

Right, this is for shared descriptors. The patch does
not work unless you set umask(0), so it's actually 0666.
By default umask is 0022, so lock file was 0644 and the
open( O_RDWR) in child fails with EPERM.

Anyhow, this is for rare cases where there is a kernel bug
with shared files and flock, and disabled unless compiled
manually with -DJK_SHM_LOCK_REOPEN.
Of course this lowers the security of the system, cause
you have /tmp/jkshmlock.XXXXX file with -rw-rw-rw

Regards,
Mladen.



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r518581 - /tomcat/connectors/trunk/jk/native/common/jk_shm.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
I mean when Apache 1.3 needs to create lockfiles,
it does so 0600...

On Mar 15, 2007, at 10:06 AM, Henri Gomez wrote:

> On iSeries it's created as 0666 (qtmhhttp:0)
>
> 2007/3/15, Jim Jagielski <ji...@jagunet.com>:
>>
>> On Mar 15, 2007, at 9:36 AM, Mladen Turk wrote:
>>
>> > Jim Jagielski wrote:
>> >> Do we *really* want 0666 on these things? Wouldn't 0644 be better?
>> >
>> > Of course. I just made that compile time enabled for faulty  
>> kernels.
>> > I'll check if the 0644 works for flock.
>> >
>>
>> It should... these are opened early enough and since
>> the fd's are shared, the subprocesses/threads should
>> still have access. Under Apache 1.3 we even have
>> them 0600 :)
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r518581 - /tomcat/connectors/trunk/jk/native/common/jk_shm.c

Posted by Henri Gomez <he...@gmail.com>.
On iSeries it's created as 0666 (qtmhhttp:0)

2007/3/15, Jim Jagielski <ji...@jagunet.com>:
>
> On Mar 15, 2007, at 9:36 AM, Mladen Turk wrote:
>
> > Jim Jagielski wrote:
> >> Do we *really* want 0666 on these things? Wouldn't 0644 be better?
> >
> > Of course. I just made that compile time enabled for faulty kernels.
> > I'll check if the 0644 works for flock.
> >
>
> It should... these are opened early enough and since
> the fd's are shared, the subprocesses/threads should
> still have access. Under Apache 1.3 we even have
> them 0600 :)
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r518581 - /tomcat/connectors/trunk/jk/native/common/jk_shm.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
On Mar 15, 2007, at 9:36 AM, Mladen Turk wrote:

> Jim Jagielski wrote:
>> Do we *really* want 0666 on these things? Wouldn't 0644 be better?
>
> Of course. I just made that compile time enabled for faulty kernels.
> I'll check if the 0644 works for flock.
>

It should... these are opened early enough and since
the fd's are shared, the subprocesses/threads should
still have access. Under Apache 1.3 we even have
them 0600 :)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r518581 - /tomcat/connectors/trunk/jk/native/common/jk_shm.c

Posted by Mladen Turk <mt...@apache.org>.
Jim Jagielski wrote:
> Do we *really* want 0666 on these things? Wouldn't 0644 be better?
> 

Of course. I just made that compile time enabled for faulty kernels.
I'll check if the 0644 works for flock.

Regards,
Mladen.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r518581 - /tomcat/connectors/trunk/jk/native/common/jk_shm.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
Do we *really* want 0666 on these things? Wouldn't 0644 be better?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r518581 - /tomcat/connectors/trunk/jk/native/common/jk_shm.c

Posted by Mladen Turk <mt...@apache.org>.
Rainer Jung wrote:
> Hi Mladen,
> 
> I didn't really read this commit, but it's Log entry reminds me of the 
> following:
> 

This is for shm lock, not for the shared memory, so the name is
irrelevant, and will be destroyed on exit.

Also, we store workers in shared memory, so any attempt to
have a shared memory between instances is tricky, because
it can lead to a total mess if the configuration is not
exactly the same (even ordering matters).

Regards,
Mladen.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r518581 - /tomcat/connectors/trunk/jk/native/common/jk_shm.c

Posted by Rainer Jung <ra...@kippdata.de>.
Hi Mladen,

I didn't really read this commit, but it's Log entry reminds me of the 
following:

We just now had a case where a user started two instances and used the 
same (default) shm file. Wouldn't it be nice to use the trick httpd uses 
for most run files, namely to add the pid of the parent to the name of 
the shm file?

I must admit, that I didn't check, if this would be easy, namely, if 
there would be a problem for the clients to find out the actual name, 
but I guess it would be easy?

Regards,

Rainer

mturk@apache.org wrote:
> Author: mturk
> Date: Thu Mar 15 04:11:20 2007
> New Revision: 518581
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=518581
> Log:
> Fix shared memory lock by using temporary file instead
> adding extension to shared memory file. This allows to
> have higher security for shared memory
> 
> Modified:
>     tomcat/connectors/trunk/jk/native/common/jk_shm.c
> 
> Modified: tomcat/connectors/trunk/jk/native/common/jk_shm.c
> URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_shm.c?view=diff&rev=518581&r1=518580&r2=518581
> ==============================================================================
> --- tomcat/connectors/trunk/jk/native/common/jk_shm.c (original)
> +++ tomcat/connectors/trunk/jk/native/common/jk_shm.c Thu Mar 15 04:11:20 2007
> @@ -59,6 +59,7 @@
>  {
>      size_t     size;
>      const char *filename;
> +    const char *lockname;
>      int        fd;
>      int        fd_lock;
>      int        attached;
> @@ -69,7 +70,7 @@
>  typedef struct jk_shm jk_shm_t;
>  
>  static const char shm_signature[] = { JK_SHM_MAGIC };
> -static jk_shm_t jk_shmem = { 0, NULL, -1, -1, 0, NULL};
> +static jk_shm_t jk_shmem = { 0, NULL, NULL, -1, -1, 0, NULL};
>  static time_t jk_workers_modified_time = 0;
>  static time_t jk_workers_access_time = 0;
>  #if defined (WIN32)
> @@ -227,44 +228,68 @@
>  #define MAP_FILE    (0)
>  #endif
>  
> -static int do_shm_open_lock(const char *fname, int attached, jk_logger_t *l)
> +static int do_shm_open_lock(int attached, jk_logger_t *l)
>  {
>      int rc;
> -    int fd;
> -    int flags = O_RDWR;
>      char flkname[256];
>      JK_TRACE_ENTER(l);
>  
> -    jk_shmem.fd_lock = -1;
> -    strcpy(flkname, fname);
> -    strcat(flkname, ".lock");
> -    if (!attached)
> -        flags |= (O_CREAT|O_TRUNC);
> -    fd = open(flkname, flags, 0666);
> -    if (fd == -1) {
> +    if (!jk_shmem.lockname) {
> +        int i;
> +        jk_shmem.fd_lock = -1;
> +        for (i = 0; i < 8) {
> +            strcpy(flkname, "/tmp/jkshmlock.XXXXXX");
> +            if (mktemp(flkname)) {
> +                jk_shmem.fd_lock = open(flkname, O_RDWR|O_CREAT|O_TRUNC, 0666);
> +                if (jk_shmem.fd_lock >= 0)
> +                    break;
> +            }
> +        }
> +        if (jk_shmem.fd_lock == -1) {
> +            rc = errno;
> +            JK_TRACE_EXIT(l);
> +            return rc;
> +        }
> +        jk_shmem.lockname = strdup(flkname);
> +        unlink(jk_shmem.lockname);
> +    }
> +    else if (attached) {
> +        jk_shmem.fd_lock = open(jk_shmem.lockname, O_RDWR, 0666);
> +        if (jk_shmem.fd_lock == -1) {
> +            rc = errno;
> +            JK_TRACE_EXIT(l);
> +            return rc;
> +        }
> +        if (JK_IS_DEBUG_LEVEL(l))
> +            jk_log(l, JK_LOG_DEBUG,
> +                   "Duplicated shared memory lock %s", jk_shmem.lockname);
>          JK_TRACE_EXIT(l);
> -        return errno;
> +        return 0;
> +    }
> +    else {
> +        
> +        
>      }
>  
>      if (!attached) {
> -        if (ftruncate(fd, 1)) {
> +        if (ftruncate(jk_shmem.fd_lock, 1)) {
>              rc = errno;
> -            close(fd);
> +            close(jk_shmem.fd_lock);
> +            jk_shmem.fd_lock = -1;
>              JK_TRACE_EXIT(l);
>              return rc;
>           }
>      }
> -    if (lseek(fd, 0, SEEK_SET) != 0) {
> +    if (lseek(jk_shmem.fd_lock, 0, SEEK_SET) != 0) {
>          rc = errno;
> -        close(fd);
> +        close(jk_shmem.fd_lock);
> +        jk_shmem.fd_lock = -1;
>          JK_TRACE_EXIT(l);
>          return rc;
>      }
> -    jk_shmem.fd_lock = fd;
> -
>      if (JK_IS_DEBUG_LEVEL(l))
>          jk_log(l, JK_LOG_DEBUG,
> -               "Opened shared memory lock %s", flkname);
> +               "Opened shared memory lock %s", jk_shmem.lockname);
>      JK_TRACE_EXIT(l);
>      return 0;
>  }
> @@ -274,7 +299,6 @@
>  {
>      int rc;
>      int fd;
> -    int flags = O_RDWR;
>      void *base;
>  
>      JK_TRACE_ENTER(l);
> @@ -283,12 +307,14 @@
>          if (!attached)
>              attached = 1;
>      }
> +    else if (attached) {
> +        /* We should already have a header
> +         * Use memory if we don't
> +         */
> +        JK_TRACE_EXIT(l);
> +        return 0;
> +    }
>      jk_shmem.filename = fname;
> -    if (attached)
> -        jk_shmem.attached = (int)getpid();
> -    else
> -        jk_shmem.attached = 0;
> -
>      jk_shmem.size = JK_SHM_ALIGN(sizeof(jk_shm_header_t) + sz);
>  
>      /* Use plain memory in case there is no file name */
> @@ -301,17 +327,16 @@
>          return 0;
>      }
>  
> -    if (!attached)
> -        flags |= (O_CREAT|O_TRUNC);
> -    fd = open(fname, flags, 0666);
> -    if (fd == -1) {
> -        jk_shmem.size = 0;
> -        JK_TRACE_EXIT(l);
> -        return errno;
> -    }
> -
>      if (!attached) {
> -        size_t size = lseek(fd, 0, SEEK_END);
> +        size_t size;
> +        jk_shmem.attached = 0;
> +        fd = open(fname, O_RDWR|O_CREAT|O_TRUNC, 0666);
> +        if (fd == -1) {
> +            jk_shmem.size = 0;
> +            JK_TRACE_EXIT(l);
> +            return errno;
> +        }
> +        size = lseek(fd, 0, SEEK_END);
>          if (size < jk_shmem.size) {
>              size = jk_shmem.size;
>              if (ftruncate(fd, jk_shmem.size)) {
> @@ -325,31 +350,27 @@
>                  jk_log(l, JK_LOG_DEBUG,
>                         "Truncated shared memory to %u", size);
>          }
> -    }
> -    if (lseek(fd, 0, SEEK_SET) != 0) {
> -        rc = errno;
> -        close(fd);
> -        jk_shmem.size = 0;
> -        JK_TRACE_EXIT(l);
> -        return rc;
> -    }
> -
> -    base = mmap((caddr_t)0, jk_shmem.size,
> -                PROT_READ | PROT_WRITE,
> -                MAP_FILE | MAP_SHARED,
> -                fd, 0);
> -    if (base == (caddr_t)MAP_FAILED || base == (caddr_t)0) {
> -        rc = errno;
> -        close(fd);
> -        jk_shmem.size = 0;
> -        JK_TRACE_EXIT(l);
> -        return rc;
> -    }
> -    jk_shmem.hdr = base;
> -    jk_shmem.fd  = fd;
> +        if (lseek(fd, 0, SEEK_SET) != 0) {
> +            rc = errno;
> +            close(fd);
> +            jk_shmem.size = 0;
> +            JK_TRACE_EXIT(l);
> +            return rc;
> +        }
>  
> -    /* Clear shared memory */
> -    if (!attached) {
> +        base = mmap((caddr_t)0, jk_shmem.size,
> +                    PROT_READ | PROT_WRITE,
> +                    MAP_FILE | MAP_SHARED,
> +                    fd, 0);
> +        if (base == (caddr_t)MAP_FAILED || base == (caddr_t)0) {
> +            rc = errno;
> +            close(fd);
> +            jk_shmem.size = 0;
> +            JK_TRACE_EXIT(l);
> +            return rc;
> +        }
> +        jk_shmem.hdr = base;
> +        jk_shmem.fd  = fd;
>          memset(jk_shmem.hdr, 0, jk_shmem.size);
>          memcpy(jk_shmem.hdr->h.data.magic, shm_signature, JK_SHM_MAGIC_SIZ);
>          jk_shmem.hdr->h.data.size = sz;
> @@ -360,11 +381,12 @@
>                     jk_shmem.size, jk_shmem.hdr->h.data.size, jk_shmem.hdr);
>      }
>      else {
> -        jk_shmem.hdr->h.data.childs++;
> +        unsigned int nchild = jk_shmem.hdr->h.data.childs + 1;
> +        jk_shmem.attached = (int)getpid();
>          if (JK_IS_DEBUG_LEVEL(l))
>              jk_log(l, JK_LOG_INFO,
>                     "Attached shared memory [%d] size=%u free=%u addr=%#lx",
> -                   jk_shmem.hdr->h.data.childs, jk_shmem.hdr->h.data.size,
> +                   nchild, jk_shmem.hdr->h.data.size,
>                     jk_shmem.hdr->h.data.size - jk_shmem.hdr->h.data.pos,
>                     jk_shmem.hdr);
>          /*
> @@ -374,20 +396,23 @@
>           * if the number of workers change between
>           * open and attach or between two attach operations.
>           */
> -        if (jk_shmem.hdr->h.data.childs > 1) {
> +        if (nchild > 1) {
>              if (JK_IS_DEBUG_LEVEL(l)) {
>                  jk_log(l, JK_LOG_DEBUG,
>                         "Reseting the shared memory for child %d",
> -                       jk_shmem.hdr->h.data.childs);
> +                       nchild);
>              }
>          }
> +        jk_shmem.hdr->h.data.childs  = nchild;
>          jk_shmem.hdr->h.data.pos     = 0;
>          jk_shmem.hdr->h.data.workers = 0;
>      }
>      JK_INIT_CS(&(jk_shmem.cs), rc);
> -    if ((rc = do_shm_open_lock(fname, attached, l))) {
> -        munmap((void *)jk_shmem.hdr, jk_shmem.size);
> -        close(jk_shmem.fd);
> +    if ((rc = do_shm_open_lock(attached, l))) {
> +        if (!attached) {
> +            munmap((void *)jk_shmem.hdr, jk_shmem.size);
> +            close(jk_shmem.fd);
> +        }
>          jk_shmem.hdr = NULL;
>          jk_shmem.fd  = -1;
>          JK_TRACE_EXIT(l);
> @@ -413,27 +438,32 @@
>      if (jk_shmem.hdr) {
>          --jk_shmem.hdr->h.data.childs;
>  
> +        if (jk_shmem.fd_lock >= 0) {
> +            close(jk_shmem.fd_lock);
> +            jk_shmem.fd_lock = -1;
> +        }
> +        JK_DELETE_CS(&(jk_shmem.cs), rc);
>          if (jk_shmem.attached) {
>              int p = (int)getpid();
> -            if (p != jk_shmem.attached) {
> +            if (p == jk_shmem.attached) {
>                  /* In case this is a forked child
>                   * do not close the shared memory.
>                   * It will be closed by the parent.
>                   */
> -                 return;
> +                jk_shmem.size = 0;
> +                jk_shmem.hdr  = NULL;
> +                jk_shmem.fd   = -1;
> +                return;
>              }
>          }
> -        if (jk_shmem.fd_lock >= 0) {
> -            close(jk_shmem.fd_lock);
> -        }
>          if (jk_shmem.fd >= 0) {
>              munmap((void *)jk_shmem.hdr, jk_shmem.size);
>              close(jk_shmem.fd);
> +            if (jk_shmem.lockname) {
> +                free(jk_shmem.lockname);
> +                jk_shmem.lockname = NULL;
> +            }
>          }
> -        jk_shmem.fd_lock = -1;
> -    }
> -    if (jk_shmem.size) {
> -        JK_DELETE_CS(&(jk_shmem.cs), rc);
>      }
>      jk_shmem.size = 0;
>      jk_shmem.hdr  = NULL;

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org