NetBSD Problem Report #30562

From thesing@cs.uni-sb.de  Mon Jun 20 08:52:01 2005
Return-Path: <thesing@cs.uni-sb.de>
Received: from uni-sb.de (uni-sb.de [134.96.252.33])
	by narn.netbsd.org (Postfix) with ESMTP id 245FA63B104
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 20 Jun 2005 08:52:00 +0000 (UTC)
Message-Id: <200506200851.j5K8puFP002953@gargoyle.cs.uni-sb.de>
Date: Mon, 20 Jun 2005 10:51:56 +0200 (CEST)
From: thesing@cs.uni-sb.de
Reply-To: thesing@cs.uni-sb.de
To: gnats-bugs@netbsd.org
Subject: qt3 doesn't handle child processes correctly
X-Send-Pr-Version: 3.95

>Number:         30562
>Category:       pkg
>Synopsis:       qt3 doesn't handle child processes in QProcess class correctly
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    pkg-manager
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jun 20 08:53:00 +0000 2005
>Closed-Date:    Tue Jun 19 11:23:52 +0000 2018
>Last-Modified:  Tue Jun 19 11:23:52 +0000 2018
>Originator:     Stephan Thesing
>Release:        NetBSD 3.99.5
>Organization:
=  Tel.: +49-681-302-5571      = Universitaet des Saarlandes =
=  Fax.: +49-681-302-3065      = Postfach 15 11 50           =
=  Compiler Research Group     = 66041 Saarbruecken          =
=  FR 6.2 - Informatik         = GERMANY                     =
>Environment:


System: NetBSD gargoyle.cs.uni-sb.de 3.99.5 NetBSD 3.99.5 (Gargoyle) #13: Thu Jun 9 12:47:43 CEST 2005 thesing@gargoyle.cs.uni-sb.de:/local/thesing/netbsd/current/obj/sys/arch/i386/compile.i386/Gargoyle i386
Architecture: i386
Machine: i386
>Description:
 The qt3-libs package under pkgsrc/x11/qt3-libs, version 3.3.4 has a Qt inherent bug
 in the handling of child processes via the QProcess class (src/kernel/qprocess_unix.cpp).
 The problem is the following: 
   The QProcess class (more exactly, the QProcessManager class) installs a signal handler
   to capture SIGCHLD for exiting child processes.  In the handler, it writes to a socket,
   which again is bound to a QNotifier which notifies the main event loop about child
   processes that have finished.
   Unfortunately, the old UNIX problem arises: it is not guaranteed that the signal handler
    will be called for _every_ child process that exited.  In fact, if multiple children
    have exited, it will be called only once for them, causing the QProcess class to loose
    notification for at least one child.  This is more severe since 2.0 as QT Apps are
    threaded and thus use scheduler activations with the native pthread library.  This means
    that the invocation of the signal handler can be delayed quite a bit, if no idle thread
    is available to take the handler. ( with PTHREAD_CONCURRENCY >1, this will also hit
      assertions in libpthread, but this is a different story).
 In one local application, which invokes a number of QProcess classes sequentially, this leads to the
  loss of child exit notification somewhere along the way.
>How-To-Repeat:
 Invoke a number of QProcesses in a QT Application and wait for them. At some point they will
  loose the notification of completed child processes...
>Fix:
    the attached patch for x11/qt3-libs does the following:
     on NetBSD >=2.0 it uses kqueue to keep track of the occurences of SIGCHLD.
     For this, a new QNotifier is added to the QProcessManager class that waits
      for events on the kqueue descriptor (kfd in the class).
     Instead of installing a signal handler, a new kqueue descriptor is opened, a
      filter for SIGCHLD is added and the notifier is bound to the kqueue descriptor.
     The handler for the kqueue descriptor determines the number of SIGCHLD signals that
      occured and writes one byte per occured signal to the old pipe, which will trigger
      the old QNotifier (once per child), which was previously triggered via the signal
      handler (once per handler invocation).
    For my local application, this fixed the problem.
    I have tested this on 3.99.5 and compile tested on 2.0.  I did not test on 1.6.x yet.

--- src/kernel/qprocess_unix.cpp.orig	2005-01-21 18:16:11.000000000 +0100
+++ src/kernel/qprocess_unix.cpp	2005-06-19 21:51:33.000000000 +0200
@@ -59,6 +59,11 @@
 #include <errno.h>
 #include <sys/types.h>

+#if defined(__NetBSD__) && __NetBSD_Version__>=200000000
+#include <sys/event.h>
+#include <sys/time.h>
+#endif
+
 #ifdef __MIPSEL__
 # ifndef SOCK_DGRAM
 #  define SOCK_DGRAM 1
@@ -187,15 +192,26 @@
 public slots:
     void removeMe();
     void sigchldHnd( int );
+#if defined(__NetBSD__) && __NetBSD_Version__>=200000000
+    void sigchldHnd2( int );
+#endif

 public:
+#if !defined(__NetBSD__) || __NetBSD_Version__<200000000
     struct sigaction oldactChld;
+#endif
     struct sigaction oldactPipe;
     QPtrList<QProc> *procList;
     int sigchldFd[2];
-
+#if defined(__NetBSD__) && __NetBSD_Version__>=200000000
+    int kfd;
+    struct kevent kevent;
+#endif
 private:
     QSocketNotifier *sn;
+#if defined(__NetBSD__) && __NetBSD_Version__>=200000000
+    QSocketNotifier *sn2;
+#endif
 };

 static void qprocess_cleanup()
@@ -253,7 +269,7 @@
 #undef BAILOUT
 #endif

-QProcessManager::QProcessManager() : sn(0)
+QProcessManager::QProcessManager() : sn(0), sn2(0)
 {
     procList = new QPtrList<QProc>;
     procList->setAutoDelete( TRUE );
@@ -261,7 +277,7 @@
     // The SIGCHLD handler writes to a socket to tell the manager that
     // something happened. This is done to get the processing in sync with the
     // event reporting.
-#ifndef Q_OS_QNX6
+#if  !defined(Q_OS_QNX6) 
     if ( ::socketpair( AF_UNIX, SOCK_STREAM, 0, sigchldFd ) ) {
 #else
     if ( qnx6SocketPairReplacement (sigchldFd) ) {
@@ -272,6 +288,40 @@
 #if defined(QT_QPROCESS_DEBUG)
 	qDebug( "QProcessManager: install socket notifier (%d)", sigchldFd[1] );
 #endif
+#if defined(__NetBSD__) && __NetBSD_Version__>=200000000
+
+	kfd=kqueue();
+        if (-1==kfd) {
+            qWarning("Cannot create kqueue filter");
+            kfd=0;
+        } else {
+          struct kfilter_mapping km;
+          int n;
+
+          km.name="EVFILT_SIGNAL";
+          if (-1==ioctl(kfd, KFILTER_BYNAME, &km)) {
+             qWarning("Getting filter number failed");
+             close(kfd);
+             kfd=0;
+          } else {
+             bzero(&kevent, sizeof(kevent));
+             kevent.ident=SIGCHLD;
+             kevent.filter=km.filter;
+             kevent.flags=EV_ADD|EV_ENABLE;
+             n=::kevent(kfd, &kevent, 1, NULL, 0, NULL);
+             if (-1==n) {
+               qWarning("Cannot add signal filter");
+               close(kfd);
+               kfd=0;
+             } else {
+               sn2 = new QSocketNotifier(kfd, QSocketNotifier::Read, this);
+               connect( sn2, SIGNAL(activated(int)),
+                        this, SLOT(sigchldHnd2(int)) );
+               sn2->setEnabled( TRUE );
+             }
+          }
+        }
+#endif
 	sn = new QSocketNotifier( sigchldFd[1],
 		QSocketNotifier::Read, this );
 	connect( sn, SIGNAL(activated(int)),
@@ -282,6 +332,7 @@
     // install a SIGCHLD handler and ignore SIGPIPE
     struct sigaction act;

+#if !defined(__NetBSD__) || __NetBSD_Version__<200000000
 #if defined(QT_QPROCESS_DEBUG)
     qDebug( "QProcessManager: install a SIGCHLD handler" );
 #endif
@@ -294,6 +345,7 @@
 #endif
     if ( sigaction( SIGCHLD, &act, &oldactChld ) != 0 )
 	qWarning( "Error installing SIGCHLD handler" );
+#endif

 #if defined(QT_QPROCESS_DEBUG)
     qDebug( "QProcessManager: install a SIGPIPE handler (SIG_IGN)" );
@@ -314,13 +366,18 @@
 	::close( sigchldFd[0] );
     if ( sigchldFd[1] != 0 )
 	::close( sigchldFd[1] );
-
+#if defined(__NetBSD__) && __NetBSD_Version__>=200000000
+    if (kfd != 0)
+        ::close( kfd );
+#endif
     // restore SIGCHLD handler
 #if defined(QT_QPROCESS_DEBUG)
     qDebug( "QProcessManager: restore old sigchild handler" );
 #endif
+#if !defined(__NetBSD__) || __NetBSD_Version__<200000000
     if ( sigaction( SIGCHLD, &oldactChld, 0 ) != 0 )
 	qWarning( "Error restoring SIGCHLD handler" );
+#endif

 #if defined(QT_QPROCESS_DEBUG)
     qDebug( "QProcessManager: restore old sigpipe handler" );
@@ -362,6 +419,29 @@
     }
 }

+#if defined(__NetBSD__) && __NetBSD_Version__>=200000000
+void QProcessManager::sigchldHnd2( int fd )
+{
+  char a;
+  int n;
+  struct timespec tm;
+
+  if (!sn2)
+    return;
+
+  tm.tv_sec=0; tm.tv_nsec=0;
+
+  n=::kevent(kfd, NULL, 0, &QProcessPrivate::procManager->kevent, 1, &tm);
+  if (1==n) {
+    a=' ';
+    for (n=QProcessPrivate::procManager->kevent.data; n>0; n--) {
+      if (0!=QProcessPrivate::procManager->sigchldFd[0])
+        ::write( QProcessPrivate::procManager->sigchldFd[0], &a, sizeof(a) );
+    }
+  }
+}
+#endif
+
 void QProcessManager::sigchldHnd( int fd )
 {
     // Disable the socket notifier to make sure that this function is not
@@ -1075,6 +1155,7 @@
 */
 bool QProcess::isRunning() const
 {
+    pid_t pres;
     if ( d->exitValuesCalculated ) {
 #if defined(QT_QPROCESS_DEBUG)
 	qDebug( "QProcess::isRunning(): FALSE (already computed)" );
@@ -1084,7 +1165,8 @@
     if ( d->proc == 0 )
 	return FALSE;
     int status;
-    if ( ::waitpid( d->proc->pid, &status, WNOHANG ) == d->proc->pid ) {
+    pres=::waitpid( d->proc->pid, &status, WNOHANG );
+    if (pres == d->proc->pid || (-1==pres && errno==ECHILD)) {
 	// compute the exit values
 	QProcess *that = (QProcess*)this; // mutable
 	that->exitNormal = WIFEXITED( status ) != 0;

>Release-Note:

>Audit-Trail:

State-Changed-From-To: open->feedback
State-Changed-By: wiz@netbsd.org
State-Changed-When: Thu, 23 Jun 2005 17:26:46 +0000
State-Changed-Why:
This bug report should be sent to the qt3 authors.
Also, it would be better if you made a configure test for kqueue and
used a symbol that gets defined if kqueue is available instead of making
this NetBSD-version related hack (FreeBSD also has kqueue).
Please let us know how you proceed, or what the qt3 authors tell you.
Thanks.


From: Stephan Thesing <thesing@cs.uni-sb.de>
To: wiz@netbsd.org
Cc: pkg-manager@netbsd.org, pkgsrc-bugs@netbsd.org,
	gnats-admin@netbsd.org
Subject: Re: pkg/30562
Date: Fri, 24 Jun 2005 15:32:25 +0200 (CEST)

 On Thu, 23 Jun 2005 wiz@netbsd.org wrote:

 > Date: Thu, 23 Jun 2005 17:26:48 +0000 (UTC)
 > From: wiz@netbsd.org
 > To: pkg-manager@netbsd.org, pkgsrc-bugs@netbsd.org, gnats-admin@netbsd.org,
 >     wiz@netbsd.org, thesing@cs.uni-sb.de
 > Subject: Re: pkg/30562
 > 
 > Synopsis: qt3 doesn't handle child processes in QProcess class correctly
 >
 > State-Changed-From-To: open->feedback
 > State-Changed-By: wiz@netbsd.org
 > State-Changed-When: Thu, 23 Jun 2005 17:26:46 +0000
 > State-Changed-Why:
 > This bug report should be sent to the qt3 authors.
 > Also, it would be better if you made a configure test for kqueue and
 > used a symbol that gets defined if kqueue is available instead of making
 > this NetBSD-version related hack (FreeBSD also has kqueue).
 > Please let us know how you proceed, or what the qt3 authors tell you.
 > Thanks.

 I did send a modified patch to Trolltech, they are looking into it.

 Best regards.....
  	Stephan


 =  Tel.: +49-681-302-5571      = Universitaet des Saarlandes =
 =  Fax.: +49-681-302-3065      = Postfach 15 11 50           =
 =  Compiler Research Group     = 66041 Saarbruecken          =
 =  FR 6.2 - Informatik         = GERMANY                     =

State-Changed-From-To: feedback->suspended
State-Changed-By: wiz@netbsd.org
State-Changed-When: Sat, 17 Sep 2005 20:03:11 +0000
State-Changed-Why:
Will hopefully be fixed in a newer version of qt3.


State-Changed-From-To: suspended->feedback
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Thu, 23 Jun 2016 18:39:08 +0000
State-Changed-Why:
Was suspended in 2005 with "Will hopefully be fixed in a newer version of qt3.
"
...so, now it's 2016. Is this still an issue?


State-Changed-From-To: feedback->open
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Tue, 14 Mar 2017 06:48:00 +0000
State-Changed-Why:
Feedback mail is bouncing. As there's a patch in here, I don't want to
just close the PR unless we're sure the patch isn't needed.


State-Changed-From-To: open->closed
State-Changed-By: maya@NetBSD.org
State-Changed-When: Tue, 19 Jun 2018 11:23:52 +0000
State-Changed-Why:
qt3 was removed. sorry the bug report ended in a weird state where it's currently unclear if any of it got applied.
Thanks for the patch & report.


>Unformatted:

Home	PR Database Search