On Fri, 15 Dec 2006 18:11:21 +0100 Bill Allombert <[EMAIL PROTECTED]> wrote:
> On Fri, Dec 15, 2006 at 12:19:35PM +0100, Raphael Hertzog wrote: > > No, I don't remember having encountered this problem, but the information > > provided by others looks convincing. > > > > I would apply the patch, check that update-menus still works according to > > your wishes and trust the others to verify that it fixed the problem. > > So far none of the people affected has been positive that it actually > fixed the bug for them. This is a problem. Sure it does, it fixes it for me. And I think I convincingly argued why it does. Mario and Yan could you please verufy that the attached patch works for you to? > Maybe we should reassign the bug to glibc. But that wouldn't fix the bug now. The patch fixes the bug without loss of functionality. It is just a bit simpler in design; less chance for bugs. IIRC, it appears that upstream is aware of the bug. > What worry me about the patch is the fact that create_lock() and > check_dpkglock() are not performed in the same order. In particular, if > create_lock() fail we exit with error 0 instead of 1 thus maybe > concealing a real error. You used to exit with 1 only if both creat_lock and check_dpkglock were false at the same time. If that is what you want, that is easy to fix. New patch attached. The locking logic is now the same as in the original. grts Tim
--- update-menus/update-menus.cc-- 2006-10-31 17:00:50.000000000 +0100
+++ update-menus/update-menus.cc 2006-12-15 20:58:04.000000000 +0100
@@ -792,77 +792,51 @@
int child;
int parentpid;
int i,r;
-
// This function checks to see if dpkg is running, and if
// so, forks into the background, to let dpkg go on installing
// packages. After dpkg finished (and wrote the new packages file),
// this function wakes up, and returns.
-
- // Writing the console output correctly is actually non-trivial.
- // The problem is that we in the following if statement
- // if(child=fork())
- // exit(0); //parent process
- // else
- // do something (and write to stdout); //child `background' process
- // want to write to stdout. But if we write to stdout there,
- // the exit(0) may already have occured, and dpkg may already
- // have started writing stuff to the console. To prevent that,
- // I use signals: the `background' process sents a signal to the
- // parent once it's written everything it wants to stdout,
- // and only after the parent received the signal it will exit(0);
- // [Oh god! (added by Bill trying to understand the fork() business)]
+ // First try to create update-menus lock
+ r = create_lock();
+
+ // Now check dpkg lock
if (check_dpkglock()) {
- sigset_t sig,oldsig;
+ // Probably another process is waiting for dpkg, exit
+ if (!r)
+ exit(0);
- // Apparently libc2 on 2.0 kernels, with threading on, blocks
- // SIGUSR1. This blocking would be inherited by children, so
- // as apt was compiled with -lpthread, this caused problems in
- // update-menus. I now get rid of that by using
- // - SIGUSR2 instead of SIGUSR1,
- // - sigprocmask to unblock SIGUSR2.
- // Either one of those solutions should be enough, though.
-
- sigemptyset(&sig);
- sigaddset(&sig,SIGUSR2);
- sigprocmask(SIG_UNBLOCK,&sig,&oldsig);
+ // OK, we need to fork, create log file name and tell user about it.
+ stdoutfile = string("/tmp/update-menus.")+itostring(getpid());
+ config.report(String::compose(_("Waiting for dpkg to finish (will fork to background).\n"
+ "(checking %1)"), DPKG_LOCKFILE),
+ configinfo::report_normal);
+ config.report(String::compose(_("Further output (if any) will appear in %1."), stdoutfile),
+ configinfo::report_normal);
- signal(SIGUSR2, exit_on_signal);
+ // Now do the fork
parentpid=getpid();
if ((child=fork())) {
if (child==-1) {
perror("update-menus: fork");
exit(1);
}
- pause();
- } else {
- r = create_lock();
- if (r) {
- stdoutfile = string("/tmp/update-menus.")+itostring(getpid());
- config.report(String::compose(_("Waiting for dpkg to finish (forking to background).\n"
- "(checking %1)"), DPKG_LOCKFILE),
- configinfo::report_normal);
- config.report(String::compose(_("Further output (if any) will appear in %1."), stdoutfile),
- configinfo::report_normal);
- // Close all fd's except the lock fd, for daemon mode.
- for (i=0;i<32;i++) {
- if (i != r)
- close(i);
- }
- kill(parentpid, SIGUSR2);
-
- while(check_dpkglock())
- sleep(2);
- } else {
- // Exit without doing anything. Kill parent too!
- kill(parentpid,SIGUSR2);
- exit(0);
+ exit(0);
+ } else {
+ // Close all fd's except the lock fd, for daemon mode.
+ for (i=0;i<32;i++) {
+ if (i != r)
+ close(i);
}
+
+ // Sit and wait until dpkg is finished ...
+ while(check_dpkglock())
+ sleep(2);
}
} else {
- r = create_lock();
+ // Locking failed, but dpkg is not locked. Maybe a real problem.
if (!r)
- exit(1);
+ exit(1);
config.report(_("Dpkg is not locking dpkg status area, good."),
configinfo::report_verbose);
signature.asc
Description: PGP signature
