[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cyrus-devel
Subject:    Re: new saslauthd/lock pidfile file
From:       Igor Brezac <igor () ypass ! net>
Date:       2003-03-31 21:52:42
[Download RAW message or body]


Rob,

More fixes.  This fixes the bizarre issue.

Index: saslauthd-main.c
===================================================================
RCS file: /cvs/src/sasl/saslauthd/saslauthd-main.c,v
retrieving revision 1.3
diff -u -r1.3 saslauthd-main.c
--- saslauthd-main.c    31 Mar 2003 20:00:53 -0000      1.3
+++ saslauthd-main.c    31 Mar 2003 21:48:48 -0000
@@ -774,7 +774,6 @@

        close(startup_pipe[1]);
        if(pid_file_lock_fd != -1) close(pid_file_lock_fd);
-       free(pid_file);
     }

     logger(L_INFO, L_FUNC, "master pid is: %lu", (unsigned long)master_pid);
@@ -852,15 +851,31 @@

        kill(-master_pid, SIGTERM);

-       /*********************************************************
-        * Tidy up and delete the pid_file. (close will release the lock)
-         * besides, we want to unlink it first anyway to avoid a race.
-        **********************************************************/
-       unlink(pid_file);
-       close(pid_fd);
+       if (flags & DETACH_TTY) {
+               /*********************************************************
+                * Tidy up and delete the pid_file. (close will release the lock)
+                * besides, we want to unlink it first anyway to avoid a race.
+                **********************************************************/
+               unlink(pid_file);
+               close(pid_fd);
+
+               if (flags & VERBOSE)
+                       logger(L_DEBUG, L_FUNC, "pid file removed: %s", pid_file);

-       if (flags & VERBOSE)
-               logger(L_DEBUG, L_FUNC, "pid file removed: %s", pid_file);
+               free(pid_file);
+       } else {
+               /*********************************************************
+                * Tidy up and delete the pid_file_lock. (close will release the lock)
+                * besides, we want to unlink it first anyway to avoid a race.
+                **********************************************************/
+               unlink(pid_file_lock);
+               close(pid_file_lock_fd);
+
+               if (flags & VERBOSE)
+                       logger(L_DEBUG, L_FUNC, "pid file lock removed: %s", pid_file_lock);
+
+               free(pid_file_lock);
+       }

        /*********************************************************
         * Cleanup the cache, if it's enabled

-Igor

On Mon, 31 Mar 2003, Rob Siemborski wrote:

> Done.
>
> On Mon, 31 Mar 2003, Igor Brezac wrote:
>
> >
> > Rob,
> >
> > More fixes.  This one should be applied to cyrus-imapd/master.c as well.
> > open(pid_file) will truncate the pid file even if another saslauthd is
> > holding the lock.  There is one more bizarre problem: saslauthd.conf file
> > is deleted every time saslauthd is stopped.
> >
> > Index: saslauthd-main.c
> > ===================================================================
> > RCS file: /cvs/src/sasl/saslauthd/saslauthd-main.c,v
> > retrieving revision 1.2
> > diff -u -r1.2 saslauthd-main.c
> > --- saslauthd-main.c    31 Mar 2003 17:13:35 -0000      1.2
> > +++ saslauthd-main.c    31 Mar 2003 19:38:40 -0000
> > @@ -692,7 +692,7 @@
> >         strcat(pid_file, PID_FILE);
> >
> >         /* Write out the pidfile */
> > -       pid_fd = open(pid_file, O_CREAT|O_TRUNC|O_RDWR, 0644);
> > +       pid_fd = open(pid_file, O_CREAT|O_RDWR, 0644);
> >         if(pid_fd == -1) {
> >             rc = errno;
> >             exit_result = 1;
> > @@ -744,8 +744,9 @@
> >
> >                 /* Write PID */
> >                 master_pid = getpid();
> > -               snprintf(buf, sizeof(buf), "%d\n", master_pid);
> > -               if (write(pid_fd, buf, strlen(buf)) == -1) {
> > +               snprintf(buf, sizeof(buf), "%-10d\n", master_pid);
> > +               if (lseek(pid_fd, 0, SEEK_SET) == -1 ||
> > +                   write(pid_fd, buf, strlen(buf)) == -1) {
> >                     int exit_result = 1;
> >                     rc = errno;
> >
> >
> > On Mon, 31 Mar 2003, Jeremy Rumpf wrote:
> >
> > > On Sunday 30 March 2003 11:37 am, Rob Siemborski wrote:
> > > > On Sat, 29 Mar 2003, Jeremy Rumpf wrote:
> > > > > That's interesting, locks are not supposed to be inherited across forks?
> > > > > Which is why detach_tty tries to relock the pid file. What's happening:
> > > > >
> > > > > 1> Process starts up
> > > > > 2> locks the pid file (to see if we can safely startup)
> > > > > 3> daemonizes into the background detaching from the controlling tty
> > > > > 4> Relocks the pid file to ensure nobody else starts a duplicate
> > > >
> > > > The race is that the parent process may not have exited by the time the
> > > > child process hits step (4), and thus it can't lock the pidfile.  The
> > > > cyrus master daemon gets around this by using a seperate lock for the
> > > > parent and child processes, and a pipe to ensure that the parent doesn't
> > > > exit.  I'll look into implementing this on monday.
> > > >
> > >
> > > Errrr, you do learn something new everyday :)
> > >
> > > Thanks,
> > > Jeremy
> > >
> >
> > --
> > Igor
> >
> >
>
> -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
> Rob Siemborski | Andrew Systems Group * Research Systems Programmer
> PGP:0x5CE32FCC | Cyert Hall 207 * rjs3@andrew.cmu.edu * 412.268.7456
> -----BEGIN GEEK CODE BLOCK----
> Version: 3.12
> GCS/IT/CM/PA d- s+: a-- C++++$ ULS++++$ P+++$ L+++(++++) E W+ N o? K-
> w O- M-- V-- PS+ PE++ Y+ PGP+ t+@ 5+++ R@ tv-@ b+ DI+++ G e h r- y?
> ------END GEEK CODE BLOCK-----
>
>

-- 
Igor


[prev in list] [next in list] [prev in thread] [next in thread]