Changeset 116

Timestamp:

Aug 11, 2006, 3:01:59 PM (19 years ago)

Author:

dmik

Message:

Kernel/Network: Changed the socket notifier activation logic so that sockets pending for activation are removed from select() until activation is processed by the main thread. This prevents deadlocks on blocking sockets (as a result of late socket notifications due to a race condition) and avoids repeated select() calls returning the same set of sockets when the event queue is not processed fast enough to handle activation. Note: this improvement is experimental. In case if it is rolled back for some reason, QServerSocket needs to be patched (by setting a non-blocking mode for sockets) to avoid GUI thread deadlocks.

Location:

trunk/src/kernel

Files:

• qeventloop_p.h (modified) (2 diffs)
• qeventloop_pm.cpp (modified) (16 diffs)

trunk/src/kernel/qeventloop_p.h

@@ -70 +70 @@
     int fd;
     fd_set *queue;
+#if defined(Q_WS_PM)
+    fd_set *active;
+#endif
 };
 
@@ -132 +135 @@
     // highest fd for all socket notifiers
     int sn_highest;
+#if defined(Q_WS_PM)
+    // fd for cancelling select()
+    int sn_cancel;
+#endif
     // 3 socket notifier types - read, write and exception
     QSockNotType sn_vec[3];

trunk/src/kernel/qeventloop_pm.cpp

@@ -658 +658 @@
 static QSemaphore ss_flag( 1 );
 
+// Note: must be called from under ss_mutex lock!
+static inline void find_highest_fd( QEventLoopPrivate *d )
+{
+    d->sn_highest = -1;
+    for ( int i = 0; i < 3; i ++ ) {
+        QPtrList<QSockNot> *list = d->sn_vec[i].list;
+        if ( list && !list->isEmpty() ) {
+            QSockNot *sn = list->first();
+            while ( sn && !FD_ISSET( sn->fd, sn->active ) )
+                sn = list->next();
+            if ( sn )
+                d->sn_highest = QMAX( d->sn_highest,  // list is fd-sorted
+                                      sn->fd );
+        }
+    }
+}
+
 void QSockSelectThread::run()
 {
@@ -680 +697 @@
             // do select
             int nfds = d->sn_highest + 1;
+            d->sn_cancel = d->sn_highest;
             ss_mutex.unlock();
             int nsel = ::select( nfds,
@@ -690 +708 @@
                 // if select says data is ready on any socket, then set
                 // the socket notifier to pending
-                int i;
-                for ( i=0; i<3; i++ ) {
+                bool activate = FALSE;
+                for ( int i = 0; i < 3; i++ ) {
                     if ( ! d->sn_vec[i].list )
                         continue;
@@ -698 +716 @@
                     while ( sn ) {
                         if ( FD_ISSET( sn->fd, &d->sn_vec[i].select_fds ) ) {
-                            // see comments inside QEventLoop::setSocketNotifierPending()
+                            // see comments in QEventLoop::setSocketNotifierPending()
                             if ( !FD_ISSET( sn->fd, sn->queue ) ) {
+                                // queue the socket activation
                                 d->sn_pending_list.insert(
                                     (rand() & 0xff) % (d->sn_pending_list.count()+1),
-                                    sn
-                                );
+                                    sn );
                                 FD_SET( sn->fd, sn->queue );
+                                // remove from enabled sockets to prevent deadlocks
+                                // on blocking sockets (and to reduce CPU load caused
+                                // by frequent select() calls) when the event queue
+                                // is not processed fast enough to handle activation
+                                FD_CLR( sn->fd, sn->active );
+                                if ( sn->fd == d->sn_highest )
+                                    find_highest_fd( d );
+                                activate = TRUE;
                             }
                         }
@@ -711 +737 @@
                 }
                 ss_mutex.unlock();
-                // post a message to activate socket notifiers 
-                qt_aux_win.post( WM_U_SEM_SELECT, 0, 0 );
+                // post a message to activate socket notifiers
+                if ( activate )
+                    qt_aux_win.post( WM_U_SEM_SELECT, 0, 0 );
             }
         } else {
+            d->sn_cancel = -1;
             // no sockets to select(), go to the idle state
             ss_mutex.unlock();
@@ -730 +758 @@
     ss_mutex.lock();
     exit = terminate;
-    if ( d->sn_highest >= 0 ) {
+    if ( d->sn_cancel >= 0 ) {
         // terminate select() execution
-        ::so_cancel( d->sn_highest );
+        ::so_cancel( d->sn_cancel );
     } else {
         // terminate the idle state by releasing the flag
@@ -768 +796 @@
 {
     d->sn_highest = -1;
+    d->sn_cancel = -1;
 
     qt_ensure_pm();
@@ -855 +884 @@
     QMutexLocker locker( &ss_mutex );
     ss_thread->cancelSelectOrIdle();
-
+    
     QPtrList<QSockNot>  *list = d->sn_vec[type].list;
-    fd_set *fds  = &d->sn_vec[type].enabled_fds;
     QSockNot *sn;
 
@@ -873 +901 @@
     sn->fd = sockfd;
     sn->queue = &d->sn_vec[type].pending_fds;
+    sn->active = &d->sn_vec[type].enabled_fds;
 
     if ( list->isEmpty() ) {
@@ -893 +922 @@
     }
 
-    FD_SET( sockfd, fds );
-    d->sn_highest = QMAX( d->sn_highest, sockfd );
+    // enable the socket only if it's not already pending
+    if ( !FD_ISSET( sockfd, sn->queue ) ) {
+        FD_SET( sockfd, sn->active );
+        d->sn_highest = QMAX( d->sn_highest, sockfd );
+    }
 #endif // QT_THREAD_SUPPORT
 }
@@ -921 +953 @@
 
     QPtrList<QSockNot> *list = d->sn_vec[type].list;
-    fd_set *fds  =  &d->sn_vec[type].enabled_fds;
     QSockNot *sn;
     if ( ! list )
@@ -931 +962 @@
         return;
 
-    FD_CLR( sockfd, fds );                      // clear fd bit
-    FD_CLR( sockfd, sn->queue );
+    // touch fd bitmaps only if there are no other notifiers for the same socket
+    // (QPtrList curiously lacks getNext()/getPrev(), so play tambourine)
+    QSockNot *next = list->next();
+    if ( next ) list->prev();
+    else list->last();
+    QSockNot *prev = list->prev();
+    if ( prev ) list->next();
+    else list->first();
+    bool unique = (!next || next->fd != sockfd) && (!prev || prev->fd != sockfd);
+    if ( unique) {
+        FD_CLR( sockfd, sn->active );           // clear fd bit
+        FD_CLR( sockfd, sn->queue );
+    }
     d->sn_pending_list.removeRef( sn );         // remove from activation list
     list->remove();                             // remove notifier found above
 
-    if ( d->sn_highest == sockfd ) {            // find highest fd
-        d->sn_highest = -1;
-        for ( int i=0; i<3; i++ ) {
-            if ( d->sn_vec[i].list && ! d->sn_vec[i].list->isEmpty() )
-                d->sn_highest = QMAX( d->sn_highest,  // list is fd-sorted
-                                      d->sn_vec[i].list->getFirst()->fd );
-        }
+    if ( unique) {
+        if ( d->sn_highest == sockfd )
+            find_highest_fd( d );
     }
 #endif // QT_THREAD_SUPPORT
@@ -967 +1005 @@
     
     QMutexLocker locker( &ss_mutex );
+    ss_thread->cancelSelectOrIdle();
 
     QPtrList<QSockNot> *list = d->sn_vec[type].list;
@@ -990 +1029 @@
             sn
         );
-        FD_SET( sn->fd, sn->queue );
+        FD_SET( sockfd, sn->queue );
+        // remove from enabled sockets, see QSockSelectThread::run()
+        FD_CLR( sockfd, sn->active );
+        if ( d->sn_highest == sockfd )
+            find_highest_fd( d );
     }
 #endif // QT_THREAD_SUPPORT
@@ -1128 +1171 @@
     
     // postpone activation if ss_thread is working with the list
-    if ( !ss_mutex.tryLock() )
+    if ( !ss_mutex.tryLock() ) {
+        qt_aux_win.post( WM_U_SEM_SELECT, 0, 0 );
         return 0;
+    }
+
+    ss_thread->cancelSelectOrIdle();
 
     // activate entries
@@ -1141 +1188 @@
         if ( FD_ISSET(sn->fd, sn->queue) ) {
             FD_CLR( sn->fd, sn->queue );
+            // reenable the socket to let it participate in the next select() 
+            FD_SET( sn->fd, sn->active );
+            d->sn_highest = QMAX( d->sn_highest, sn->fd );
             QApplication::sendEvent( sn->obj, &event );
             n_act++;