Changeset 5338 for pjproject/trunk/pjlib/src/pj/ssl_sock_ossl.c

Timestamp:

Jun 8, 2016 2:55:24 AM (8 years ago)

Author:

ming

Message:

Close #1930: Race condition in OpenSSL socket

A workaround to solve the race condition based on ticket #985.

File:

• pjproject/trunk/pjlib/src/pj/ssl_sock_ossl.c (modified) (6 diffs)

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

@@ -38 +38 @@
 #define THIS_FILE               "ssl_sock_ossl.c"
 
-/* Workaround for ticket #985 */
-#define DELAYED_CLOSE_TIMEOUT   200
+/* Workaround for ticket #985 and #1930 */
+#ifndef PJ_SSL_SOCK_DELAYED_CLOSE_TIMEOUT
+#   define PJ_SSL_SOCK_DELAYED_CLOSE_TIMEOUT    500
+#endif
 
 /* 
@@ -808 +810 @@
 
 
-/* Reset SSL socket state */
-static void reset_ssl_sock_state(pj_ssl_sock_t *ssock)
-{
-    ssock->ssl_state = SSL_STATE_NULL;
-
-    destroy_ssl(ssock);
-
+/* Close sockets */
+static void close_sockets(pj_ssl_sock_t *ssock)
+{
     if (ssock->asock) {
         pj_activesock_close(ssock->asock);
@@ -824 +822 @@
         ssock->sock = PJ_INVALID_SOCKET;
     }
+}
+
+
+/* Reset SSL socket state */
+static void reset_ssl_sock_state(pj_ssl_sock_t *ssock)
+{
+    ssock->ssl_state = SSL_STATE_NULL;
+
+    destroy_ssl(ssock);
+
+    close_sockets(ssock);
 
     /* Upon error, OpenSSL may leave any error description in the thread 
@@ -1193 +1202 @@
                       errmsg));
 
-            /* Workaround for ticket #985 */
-#if (defined(PJ_WIN32) && PJ_WIN32!=0) || (defined(PJ_WIN64) && PJ_WIN64!=0)
+            /* Originally, this is a workaround for ticket #985. However,
+             * a race condition may occur in multiple worker threads
+             * environment when we are destroying SSL objects while other
+             * threads are still accessing them.
+             * Please see ticket #1930 for more info.
+             */
+#if 1 //(defined(PJ_WIN32) && PJ_WIN32!=0)||(defined(PJ_WIN64) && PJ_WIN64!=0)
             if (ssock->param.timer_heap) {
-                pj_time_val interval = {0, DELAYED_CLOSE_TIMEOUT};
-
-                reset_ssl_sock_state(ssock);
-
+                pj_time_val interval = {0, PJ_SSL_SOCK_DELAYED_CLOSE_TIMEOUT};
+
+                ssock->ssl_state = SSL_STATE_NULL;
+                close_sockets(ssock);
+
+                if (ssock->timer.id != TIMER_NONE) {
+                    pj_timer_heap_cancel(ssock->param.timer_heap,
+                                         &ssock->timer);
+                }
                 ssock->timer.id = TIMER_CLOSE;
                 pj_time_val_normalize(&interval);
@@ -1205 +1224 @@
                                            &ssock->timer, &interval) != 0)
                 {
+                    PJ_LOG(3,(ssock->pool->obj_name, "Failed to schedule "
+                              "a delayed close. Race condition may occur."));
                     ssock->timer.id = TIMER_NONE;
                     pj_ssl_sock_close(ssock);
                 }
-            } else 
-#endif  /* PJ_WIN32 */
+            } 
+#else
             {
                 pj_ssl_sock_close(ssock);
             }
+#endif
+
             return PJ_FALSE;
         }
@@ -2216 +2239 @@
     pj_ssl_sock_param_copy(pool, &ssock->param, param);
     ssock->param.read_buffer_size = ((ssock->param.read_buffer_size+7)>>3)<<3;
+    if (!ssock->param.timer_heap) {
+        PJ_LOG(3,(ssock->pool->obj_name, "Warning: timer heap is not "
+                  "available. It is recommended to supply one to avoid "
+                  "a race condition if more than one worker threads "
+                  "are used."));
+    }
 
     /* Finally */