From: Panu Matilainen Date: Tue, 3 May 2011 08:19:37 +0000 (+0300) Subject: RIP rpmsqFork() + rpmsqWait() and the related bits X-Git-Tag: tznext/4.11.0.1.tizen20130304~1161 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=269df02ae1fc3955bee4e5f471b1172c04c714e2;p=tools%2Flibrpm-tizen.git RIP rpmsqFork() + rpmsqWait() and the related bits - Also remove additional thread protection: we're not supporting threads anywhere else either. If/when thread-protection is added, this is ulikely to be the first place anyway... --- diff --git a/rpmio/rpmsq.c b/rpmio/rpmsq.c index 4e69ad0..bfd4db0 100644 --- a/rpmio/rpmsq.c +++ b/rpmio/rpmsq.c @@ -6,100 +6,20 @@ #include #include -#include -#include #include #include -#include - -/* XXX suggested in bugzilla #159024 */ -#if PTHREAD_MUTEX_DEFAULT != PTHREAD_MUTEX_NORMAL - #error RPM expects PTHREAD_MUTEX_DEFAULT == PTHREAD_MUTEX_NORMAL -#endif - -#ifndef PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP -static pthread_mutex_t rpmsigTbl_lock = PTHREAD_MUTEX_INITIALIZER; -#else -static pthread_mutex_t rpmsigTbl_lock = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP; -#endif - -#define DO_LOCK() pthread_mutex_lock(&rpmsigTbl_lock); -#define DO_UNLOCK() pthread_mutex_unlock(&rpmsigTbl_lock); #define ADD_REF(__tbl) (__tbl)->active++ #define SUB_REF(__tbl) --(__tbl)->active -#define ME() ((void *)pthread_self()) - -#define _RPMSQ_INTERNAL #include #include "debug.h" -static struct rpmsqElem rpmsqRock; - -static rpmsq rpmsqQueue = &rpmsqRock; - -/** \ingroup rpmsq - * Insert node into from queue. - * @param elem node to link - * @param prev previous node from queue - * @return 0 on success - */ -static int rpmsqInsert(void * elem, void * prev) -{ - rpmsq sq = (rpmsq) elem; - int ret = -1; - - if (sq != NULL) { - ret = sighold(SIGCHLD); - if (ret == 0) { - sq->child = 0; - sq->reaped = 0; - sq->status = 0; - sq->reaper = 1; - sq->pipes[0] = sq->pipes[1] = -1; - - sq->id = ME(); - ret = pthread_mutex_init(&sq->mutex, NULL); - insque(elem, (prev != NULL ? prev : rpmsqQueue)); - ret = sigrelse(SIGCHLD); - } - } - return ret; -} - -/** \ingroup rpmsq - * Remove node from queue. - * @param elem node to link - * @return 0 on success - */ -static int rpmsqRemove(void * elem) -{ - rpmsq sq = (rpmsq) elem; - int ret = -1; - - if (elem != NULL) { - ret = sighold (SIGCHLD); - if (ret == 0) { - remque(elem); - - /* Unlock the mutex and then destroy it */ - if((ret = pthread_mutex_unlock(&sq->mutex)) == 0) - ret = pthread_mutex_destroy(&sq->mutex); - - sq->id = NULL; - if (sq->pipes[1]) ret = close(sq->pipes[1]); - if (sq->pipes[0]) ret = close(sq->pipes[0]); - sq->pipes[0] = sq->pipes[1] = -1; - ret = sigrelse(SIGCHLD); - } - } - return ret; -} - static sigset_t rpmsqCaught; +typedef struct rpmsig_s * rpmsig; + static struct rpmsig_s { int signum; rpmsqAction_t handler; @@ -110,8 +30,6 @@ static struct rpmsig_s { #define rpmsigTbl_sigint (&rpmsigTbl[0]) { SIGQUIT, rpmsqAction }, #define rpmsigTbl_sigquit (&rpmsigTbl[1]) - { SIGCHLD, rpmsqAction }, -#define rpmsigTbl_sigchld (&rpmsigTbl[2]) { SIGHUP, rpmsqAction }, #define rpmsigTbl_sighup (&rpmsigTbl[3]) { SIGTERM, rpmsqAction }, @@ -140,44 +58,6 @@ void rpmsqAction(int signum) continue; (void) sigaddset(&rpmsqCaught, signum); - - switch (signum) { - case SIGCHLD: - while (1) { - rpmsq sq; - int status = 0; - pid_t reaped = waitpid(0, &status, WNOHANG); - - /* XXX errno set to ECHILD/EINVAL/EINTR. */ - if (reaped <= 0) - break; - - /* XXX insque(3)/remque(3) are dequeue, not ring. */ - for (sq = rpmsqQueue->q_forw; - sq != NULL && sq != rpmsqQueue; - sq = sq->q_forw) - { - int ret; - - if (sq->child != reaped) - continue; - sq->reaped = reaped; - sq->status = status; - - /* Unlock the mutex. The waiter will then be able to - * aquire the lock. - * - * XXX: jbj, wtd, if this fails? - */ - ret = pthread_mutex_unlock(&sq->mutex); - - break; - } - } - break; - default: - break; - } break; } errno = save; @@ -190,9 +70,6 @@ int rpmsqEnable(int signum, rpmsqAction_t handler) rpmsig tbl; int ret = -1; - (void) DO_LOCK (); - if (rpmsqQueue->id == NULL) - rpmsqQueue->id = ME(); for (tbl = rpmsigTbl; tbl->signum >= 0; tbl++) { if (tblsignum != tbl->signum) continue; @@ -232,135 +109,6 @@ int rpmsqEnable(int signum, rpmsqAction_t handler) ret = tbl->active; break; } - (void) DO_UNLOCK (); return ret; } -pid_t rpmsqFork(rpmsq sq) -{ - pid_t pid; - int xx; - int nothreads = 0; /* XXX: Shouldn't this be a global? */ - - if (sq->reaper) { - xx = rpmsqInsert(sq, NULL); - xx = rpmsqEnable(SIGCHLD, NULL); - } - - xx = pipe(sq->pipes); - - xx = sighold(SIGCHLD); - - /* - * Initialize the cond var mutex. We have to aquire the lock we - * use for the condition before we fork. Otherwise it is possible for - * the child to exit, we get sigchild and the sig handler to send - * the condition signal before we are waiting on the condition. - */ - if (!nothreads) { - if(pthread_mutex_lock(&sq->mutex)) { - /* Yack we did not get the lock, lets just give up */ - xx = close(sq->pipes[0]); - xx = close(sq->pipes[1]); - sq->pipes[0] = sq->pipes[1] = -1; - goto out; - } - } - - pid = fork(); - if (pid < (pid_t) 0) { /* fork failed. */ - sq->child = (pid_t)-1; - xx = close(sq->pipes[0]); - xx = close(sq->pipes[1]); - sq->pipes[0] = sq->pipes[1] = -1; - goto out; - } else if (pid == (pid_t) 0) { /* Child. */ - int yy; - - /* Block to permit parent time to wait. */ - xx = close(sq->pipes[1]); - xx = read(sq->pipes[0], &yy, sizeof(yy)); - xx = close(sq->pipes[0]); - sq->pipes[0] = sq->pipes[1] = -1; - } else { /* Parent. */ - sq->child = pid; - } - -out: - xx = sigrelse(SIGCHLD); - return sq->child; -} - -/** - * Wait for child process to be reaped, and unregister SIGCHLD handler. - * @todo Rewrite to use waitpid on helper thread. - * @param sq scriptlet queue element - * @return 0 on success - */ -static int rpmsqWaitUnregister(rpmsq sq) -{ - int nothreads = 0; - int ret = 0; - int xx; - - /* Protect sq->reaped from handler changes. */ - ret = sighold(SIGCHLD); - - /* Start the child, linux often runs child before parent. */ - if (sq->pipes[0] >= 0) - xx = close(sq->pipes[0]); - if (sq->pipes[1] >= 0) - xx = close(sq->pipes[1]); - sq->pipes[0] = sq->pipes[1] = -1; - - /* Put a stopwatch on the time spent waiting to measure performance gain. */ - (void) rpmswEnter(&sq->op, -1); - - /* Wait for handler to receive SIGCHLD. */ - while (ret == 0 && sq->reaped != sq->child) { - if (nothreads) - /* Note that sigpause re-enables SIGCHLD. */ - ret = sigpause(SIGCHLD); - else { - xx = sigrelse(SIGCHLD); - - /* - * We start before the fork with this mutex locked; - * The only one that unlocks this the signal handler. - * So if we get the lock the child has been reaped. - */ - ret = pthread_mutex_lock(&sq->mutex); - xx = sighold(SIGCHLD); - } - } - - /* Accumulate stopwatch time spent waiting, potential performance gain. */ - sq->ms_scriptlets += rpmswExit(&sq->op, -1)/1000; - - xx = sigrelse(SIGCHLD); - - /* Remove processed SIGCHLD item from queue. */ - xx = rpmsqRemove(sq); - - /* Disable SIGCHLD handler on refcount == 0. */ - xx = rpmsqEnable(-SIGCHLD, NULL); - - return ret; -} - -pid_t rpmsqWait(rpmsq sq) -{ - if (sq->reaper) { - (void) rpmsqWaitUnregister(sq); - } else { - pid_t reaped; - int status; - do { - reaped = waitpid(sq->child, &status, 0); - } while (reaped >= 0 && reaped != sq->child); - sq->reaped = reaped; - sq->status = status; - } - - return sq->reaped; -} diff --git a/rpmio/rpmsq.h b/rpmio/rpmsq.h index 90087e0..30cb35d 100644 --- a/rpmio/rpmsq.h +++ b/rpmio/rpmsq.h @@ -5,24 +5,8 @@ * \file rpmio/rpmsq.h * */ - #include #include -#if defined(_RPMSQ_INTERNAL) -#include -#endif - -#ifdef __cplusplus -extern "C" { -#endif - -/** \ingroup rpmsq - */ -typedef struct rpmsig_s * rpmsig; - -/** \ingroup rpmsq - */ -typedef struct rpmsqElem * rpmsq; /** \ingroup rpmsq * Default signal handler prototype. @@ -36,27 +20,6 @@ typedef void (*rpmsqAction_t) (int signum, siginfo_t * info, void * context); typedef void (*rpmsqAction_t) (int signum); #endif -/* XXX make this fully opaque? */ -#if defined(_RPMSQ_INTERNAL) -/** - * SIGCHLD queue element. - */ -struct rpmsqElem { - struct rpmsqElem * q_forw; /*!< for use by insque(3)/remque(3). */ - struct rpmsqElem * q_back; - pid_t child; /*!< Currently running child. */ - volatile pid_t reaped; /*!< Reaped waitpid(3) return. */ - volatile int status; /*!< Reaped waitpid(3) status. */ - struct rpmop_s op; /*!< Scriptlet operation timestamp; */ - rpmtime_t ms_scriptlets; /*!< Accumulated script duration (msecs). */ - int reaper; /*!< Register SIGCHLD handler? */ - int pipes[2]; /*!< Parent/child interlock. */ - void * id; /*!< Blocking thread id (pthread_t). */ - pthread_mutex_t mutex; /*!< Signal delivery to thread condvar. */ - pthread_cond_t cond; -}; -#endif /* _RPMSQ_INTERNAL */ - /** \ingroup rpmsq * Test if given signal has been caught (while signals blocked). * Similar to sigismember() but operates on internal signal queue. @@ -85,20 +48,6 @@ void rpmsqAction(int signum); */ int rpmsqEnable(int signum, rpmsqAction_t handler); -/** \ingroup rpmsq - * Fork a child process. - * @param sq scriptlet queue element - * @return fork(2) pid - */ -pid_t rpmsqFork(rpmsq sq); - -/** \ingroup rpmsq - * Wait for child process to be reaped. - * @param sq scriptlet queue element - * @return reaped child pid - */ -pid_t rpmsqWait(rpmsq sq); - #ifdef __cplusplus } #endif