Changeset 5680

Timestamp:

Nov 3, 2017 6:54:54 AM (7 years ago)

Author:

nanang

Message:

Fix #2055:

• Updated active socket to avoid double ioqueue key unregistration.
• Updated ioqueue to avoid double ioq key unregistration (select & epoll only, winnt & uwp does not seem to be affected).
• Added check for potential 'negative' ioqueue->count.

Location:

pjproject/trunk/pjlib/src/pj

Files:

• activesock.c (modified) (2 diffs)
• ioqueue_epoll.c (modified) (2 diffs)
• ioqueue_select.c (modified) (1 diff)

pjproject/trunk/pjlib/src/pj/activesock.c

@@ -297 +297 @@
 PJ_DEF(pj_status_t) pj_activesock_close(pj_activesock_t *asock)
 {
+    pj_ioqueue_key_t *key;
+    pj_bool_t unregister = PJ_FALSE;
+
     PJ_ASSERT_RETURN(asock, PJ_EINVAL);
     asock->shutdown = SHUT_RX | SHUT_TX;
-    if (asock->key) {
-        pj_ioqueue_unregister(asock->key);
+
+    /* Avoid double unregistration on the key */
+    key = asock->key;
+    if (key) {
+        pj_ioqueue_lock_key(key);
+        unregister = (asock->key != NULL);
+        asock->key = NULL;
+        pj_ioqueue_unlock_key(key);
+    }
+
+    if (unregister) {
+        pj_ioqueue_unregister(key);
 
 #if defined(PJ_IPHONE_OS_HAS_MULTITASKING_SUPPORT) && \
@@ -306 +319 @@
         activesock_destroy_iphone_os_stream(asock);
 #endif  
-
-        asock->key = NULL;
     }
     return PJ_SUCCESS;

pjproject/trunk/pjlib/src/pj/ioqueue_epoll.c

@@ -517 +517 @@
     pj_ioqueue_lock_key(key);
 
+    /* Best effort to avoid double key-unregistration */
+    if (IS_CLOSING(key)) {
+        pj_ioqueue_unlock_key(key);
+        return PJ_SUCCESS;
+    }
+
     /* Also lock ioqueue */
     pj_lock_acquire(ioqueue->lock);
 
-    pj_assert(ioqueue->count > 0);
-    --ioqueue->count;
+    /* Avoid "negative" ioqueue count */
+    if (ioqueue->count > 0) {
+        --ioqueue->count;
+    } else {
+        /* If this happens, very likely there is double unregistration
+         * of a key.
+         */
+        pj_assert(!"Bad ioqueue count in key unregistration!");
+        PJ_LOG(1,(THIS_FILE, "Bad ioqueue count in key unregistration!"));
+    }
+
 #if !PJ_IOQUEUE_HAS_SAFE_UNREG
     pj_list_erase(key);
@@ -532 +547 @@
         pj_status_t rc = pj_get_os_error();
         pj_lock_release(ioqueue->lock);
+        pj_ioqueue_unlock_key(key);
         return rc;
     }

pjproject/trunk/pjlib/src/pj/ioqueue_select.c

@@ -479 +479 @@
     pj_ioqueue_lock_key(key);
 
+    /* Best effort to avoid double key-unregistration */
+    if (IS_CLOSING(key)) {
+        pj_ioqueue_unlock_key(key);
+        return PJ_SUCCESS;
+    }
+
     /* Also lock ioqueue */
     pj_lock_acquire(ioqueue->lock);
 
-    pj_assert(ioqueue->count > 0);
-    --ioqueue->count;
+    /* Avoid "negative" ioqueue count */
+    if (ioqueue->count > 0) {
+        --ioqueue->count;
+    } else {
+        /* If this happens, very likely there is double unregistration
+         * of a key.
+         */
+        pj_assert(!"Bad ioqueue count in key unregistration!");
+        PJ_LOG(1,(THIS_FILE, "Bad ioqueue count in key unregistration!"));
+    }
+
 #if !PJ_IOQUEUE_HAS_SAFE_UNREG
     /* Ticket #520, key will be erased more than once */