On Tue, Mar 27, 2007 at 11:53:42PM +0200, Jeroen van Wolffelaar wrote:
> On Tue, Mar 27, 2007 at 10:54:58PM +0200, José Luis Tallón wrote:
> > Jeroen van Wolffelaar wrote:
> > > Package: imapproxy
> > > Version: 1.2.4-10
> > > Severity: important
> > >
> > > The pid-handling of imapproxy is pretty fragile, as documented in
> > > #369020 amongst others. The current workaround of writing a new pidfile
> > > after start based on 'ps ax' output is, eh, fragile at best, and
> > > actually pretty bad.
> > >
> > > The proper solution would be to patch imapproxy so that it writes out a
> > > pidfile itself, like proper daemons should.
> > Fixed. Will submit the patch ASAP
>
> Why didn't you attach it? I'm pretty busy tomorrow, and want to get this
> fixed ASAP. I mean, the plan is to release this weekend...
NMU'd, based on your patch. See attachment for diff.
--Jeroen
--
Jeroen van Wolffelaar
[EMAIL PROTECTED] (also for Jabber & MSN; ICQ: 33944357)
http://Jeroen.A-Eskwadraat.nl
diff -u up-imapproxy-1.2.4/include/imapproxy.h up-imapproxy-1.2.4/include/imapproxy.h
--- up-imapproxy-1.2.4/include/imapproxy.h
+++ up-imapproxy-1.2.4/include/imapproxy.h
@@ -172,6 +172,10 @@
#define DEFAULT_CONFIG_FILE "/etc/imapproxy.conf"
#endif
+#ifndef DEFAULT_PID_FILE
+#define DEFAULT_PID_FILE "/var/run/imapproxy.pid"
+#endif
+
#define LITERAL_PASSWORD 1
#define NON_LITERAL_PASSWORD 0
#define UNSELECT_SUPPORTED 1
diff -u up-imapproxy-1.2.4/src/main.c up-imapproxy-1.2.4/src/main.c
--- up-imapproxy-1.2.4/src/main.c
+++ up-imapproxy-1.2.4/src/main.c
@@ -242,6 +242,7 @@
static int ParseBannerAndCapability( char *, unsigned int,
char *, unsigned int );
static void ServerInit( void );
+static void Daemonize( const char* );
static void Usage( void );
@@ -265,13 +266,13 @@
pthread_attr_t attr; /* generic thread attribute struct */
int rc, i, fd;
unsigned int ui;
- pid_t pid; /* used just for a fork call */
struct linger lingerstruct; /* for the socket reuse stuff */
int flag; /* for the socket reuse stuff */
ICC_Struct *ICC_tptr;
extern char *optarg;
extern int optind;
char ConfigFile[ MAXPATHLEN ]; /* path to our config file */
+ char PidFile[ MAXPATHLEN ]; /* path to our pidfile */
#ifdef HAVE_LIBWRAP
struct request_info r; /* request struct for libwrap */
#endif
@@ -279,6 +280,9 @@
flag = 1;
ConfigFile[0] = '\0';
+ /* initialize to default */
+ strncpy( PidFile, DEFAULT_PID_FILE, sizeof PidFile -1 );
+
/*
* Ignore signals we don't want to die from but we don't care enough
* about to catch.
@@ -287,7 +291,7 @@
signal( SIGHUP, SIG_IGN );
- while (( i = getopt( argc, argv, "f:h" ) ) != EOF )
+ while (( i = getopt( argc, argv, "f:p:h" ) ) != EOF )
{
switch( i )
{
@@ -298,7 +302,14 @@
syslog( LOG_INFO, "%s: Using configuration file '%s'",
fn, ConfigFile );
break;
-
+ case 'p':
+ /* user specified a pidfile */
+ strncpy( PidFile, optarg, sizeof PidFile -1 );
+ PidFile[ sizeof PidFile - 1 ] = '\0';
+ syslog( LOG_INFO, "%s: Using pidfile '%s'",
+ fn, PidFile );
+ break;
+
case 'h':
Usage();
exit( 0 );
@@ -403,58 +414,7 @@
ServerInit();
- /* detach from our parent if necessary */
- if (! (getppid() == 1) && ( ! PC_Struct.foreground_mode ) )
- {
- syslog( LOG_INFO, "%s: Configured to run in background mode.", fn );
-
- if ( (pid = fork()) < 0)
- {
- syslog(LOG_ERR, "%s: initial call to fork() failed: %s", fn, strerror(errno));
- exit( 1 );
- }
- else if ( pid > 0)
- {
- exit( 0 );
- }
-
- if (setsid() == -1)
- {
- syslog(LOG_WARNING, "%s: setsid() failed: %s",
- fn, strerror(errno));
- }
- if ( (pid = fork()) < 0)
- {
- syslog(LOG_ERR, "%s: secondary call to fork() failed: %s", fn,
- strerror(errno));
- exit( 1 );
- }
- else if ( pid > 0)
- {
- exit( 0 );
- }
- if( chdir("/") < 0 )
- {
- syslog(LOG_ERR, "%s: chdir(\"/\") failed: %s", fn,
- strerror(errno));
- exit( 1 );
- }
- if( (i=open("/dev/null",O_RDWR)) < 0 )
- {
- syslog(LOG_ERR, "%s: open(\"/dev/null\") failed: %s", fn,
- strerror(errno));
- exit( 1 );
- }
- close(2); dup(i);
- close(1); dup(i);
- close(0); dup(i);
- close(i);
- }
- else
- {
- syslog( LOG_INFO, "%s: Configured to run in foreground mode.", fn );
- }
-
+ Daemonize( PidFile );
SetBannerAndCapability();
@@ -723,7 +683,96 @@
* Function definitions.
*/
+/*++
+ * Function: Daemonize
+ *
+ * Purpose: Daemonize, closing all unneeded descriptors.
+ * Write the daemon's PID into 'pidfile'
+ *
+ * Parameters: pidfile -- where to write our PID.
+ *
+ * Returns: nada -- exits on error
+ *
+ * Authors: Jose Luis Tallon <[EMAIL PROTECTED]>
+ *
+ * Notes:
+ *--
+ */
+void Daemonize( const char* pidfile )
+{
+ const char* fn="Daemonize()";
+ FILE* fp=NULL;
+ pid_t pid; /* used just for a fork call */
+ int i;
+
+ if ( PC_Struct.foreground_mode ) {
+ syslog( LOG_INFO, "%s: Configured to run in foreground mode.", fn );
+ return;
+ }
+
+ /* detach from our parent */
+ syslog( LOG_INFO, "%s: Configured to run in background mode.", fn );
+ if (getppid() == 1) {
+ syslog( LOG_INFO, "%s: Forking unneeded, continuing in foreground mode anyway", fn );
+ } else {
+
+ if ( (pid = fork()) < 0)
+ {
+ syslog(LOG_ERR, "%s: initial call to fork() failed: %s", fn, strerror(errno));
+ exit( 1 );
+ }
+ else if ( pid > 0)
+ {
+ exit( 0 );
+ }
+
+ if (setsid() == -1)
+ {
+ syslog(LOG_WARNING, "%s: setsid() failed: %s",
+ fn, strerror(errno));
+ }
+ if ( (pid = fork()) < 0)
+ {
+ syslog(LOG_ERR, "%s: secondary call to fork() failed: %s", fn,
+ strerror(errno));
+ exit( 1 );
+ }
+ else if ( pid > 0)
+ {
+ exit( 0 );
+ }
+ }
+ if ( (fp=fopen(pidfile,"wt")) == NULL )
+ {
+ syslog(LOG_ERR, "%s: creating pidfile '%s' failed: %s", fn,
+ pidfile, strerror(errno));
+ exit(1);
+ }
+ if( fprintf(fp, "%u\n", (unsigned)getpid()) < 0 )
+ {
+ syslog(LOG_ERR, "%s: fprintf on pidfile failed: %s", fn,
+ strerror(errno));
+ exit(1);
+ }
+ fclose(fp);
+ if( chdir("/") < 0 )
+ {
+ syslog(LOG_ERR, "%s: chdir(\"/\") failed: %s", fn,
+ strerror(errno));
+ exit( 1 );
+ }
+ if( (i=open("/dev/null",O_RDWR)) < 0 )
+ {
+ syslog(LOG_ERR, "%s: open(\"/dev/null\") failed: %s", fn,
+ strerror(errno));
+ exit( 1 );
+ }
+ close(2); dup(i);
+ close(1); dup(i);
+ close(0); dup(i);
+ close(i);
+}
/*++
* Function: Usage
@@ -741,7 +790,7 @@
*/
void Usage( void )
{
- printf("Usage: %s [-f config filename] [-h]\n", PGM );
+ printf("Usage: %s [-f config filename] [-p pidfile] [-h]\n", PGM );
return;
}
diff -u up-imapproxy-1.2.4/debian/manpages/imapproxyd.8 up-imapproxy-1.2.4/debian/manpages/imapproxyd.8
--- up-imapproxy-1.2.4/debian/manpages/imapproxyd.8
+++ up-imapproxy-1.2.4/debian/manpages/imapproxyd.8
@@ -15,6 +15,11 @@
.B -f
.RI
<config file name> ]
+.RI
+[
+.B -p
+.RI
+<pidfile name> ]
.br
.SH DESCRIPTION
This manual page documents briefly the
@@ -41,6 +46,12 @@
This can be changed by using the
.I -f
option
+.PP
+Unless foreground_mode has been enabled, UP-ImapProxy will write its PID to to
+a PID-file. It defaults to /var/run/imapproxy.pid, but can be overridden with
+the
+.I -p
+option
.SH SEE ALSO
.BR pimpstat (8),
.br
@@ -53 +64 @@
-
\ No newline at end of file
+
diff -u up-imapproxy-1.2.4/debian/changelog up-imapproxy-1.2.4/debian/changelog
--- up-imapproxy-1.2.4/debian/changelog
+++ up-imapproxy-1.2.4/debian/changelog
@@ -1,3 +1,20 @@
+up-imapproxy (1.2.4-10.1) unstable; urgency=high
+
+ * NMU to fix RC bug
+
+ [ Jose Luis Tallon ]
+ * Re-starting imapproxy failed when it was already running.
+ - Fixed imapproxy to write a pidfile itself
+ Start-stop-daemon can now interact with it properly (Closes: #415954)
+
+ [ Jeroen van Wolffelaar ]
+ * Document -p in manpage
+ * Patch cleanup
+ * Add code to initscript to not attempt to start a second version if already
+ running, and to not fail if imapproxy is no longer running (also #415954)
+
+ -- Jeroen van Wolffelaar <[EMAIL PROTECTED]> Wed, 28 Mar 2007 04:04:55 +0200
+
up-imapproxy (1.2.4-10) unstable; urgency=high
* Bashism in initscript made upgrade sequence fail when sh was not bash
diff -u up-imapproxy-1.2.4/debian/imapproxy.init.d up-imapproxy-1.2.4/debian/imapproxy.init.d
--- up-imapproxy-1.2.4/debian/imapproxy.init.d
+++ up-imapproxy-1.2.4/debian/imapproxy.init.d
@@ -43,59 +43,41 @@
exit 0;
fi
-check_running ()
-{
- proc_cmdline="/proc/`cat $PIDFILE`/cmdline"
-
- if [ -r $proc_cmdline ]; then
- grep -q $DAEMON $proc_cmdline >/dev/null \
- && return 0
- else
- sleep 1
- psline="`ps ax | grep imapproxyd | grep -v grep`"
- if [ -n $psline ]; then
- echo `echo $psline | cut -d ' ' -f 1` > $PIDFILE
- return 0
- else
- return 1
- fi
- fi
- return 1
-}
case "$1" in
start)
+ if [ -f $PIDFILE ] && [ -e /proc/`cat $PIDFILE`/status ]; then
+ echo "Already started: $NAME."
+ exit 0
+ fi
echo -n "Starting $DESC: "
- start-stop-daemon --start --quiet --background \
- --make-pidfile --pidfile=$PIDFILE \
+ start-stop-daemon --start --quiet --pidfile=$PIDFILE \
--exec $DAEMON -- $ARGS
- ssd_ret=$?
sleep 1
- if [ $ssd_ret==0 ] && check_running ; then
- echo "$NAME."
- else
- echo "Failed to start $NAME. Check logs for details."
- exit 1
- fi
+ echo "$NAME."
;;
-
+
stop)
+ if [ ! -f $PIDFILE ]; then
+ echo "Not running: $NAME."
+ exit 0
+ fi
+ if [ ! -e /proc/`cat $PIDFILE`/status ]; then
+ rm -f $PIDFILE
+ echo "Not running: $NAME."
+ exit 0
+ fi
echo -n "Stopping $DESC: "
start-stop-daemon --oknodo --stop --quiet --pidfile=$PIDFILE \
--exec $DAEMON -- $ARGS 2>&1 | grep warning || true > /dev/null
- sleep 1
- if [ -n "`ps ax | grep imapproxyd | grep -v grep`" ]; then
- test -r $PIDFILE && kill -15 `cat $PIDFILE` 2>&1 \
- && rm -f $PIDFILE
- fi
- echo "$NAME."
-
+ rm -f $PIDFILE
+ echo "$NAME."
;;
restart|force-reload)
echo "Restarting $DESC: "
$0 stop
- sleep 1
+ sleep 2 # not gratuituous
$0 start
;;
*)
