]> git.ipfire.org Git - thirdparty/openldap.git/commitdiff
ITS#9600 Do not hold locks while calling into back-monitor
authorOndřej Kuzník <okuznik@symas.com>
Thu, 23 Sep 2021 09:17:18 +0000 (10:17 +0100)
committerOndřej Kuzník <okuznik@symas.com>
Tue, 5 Oct 2021 10:05:25 +0000 (11:05 +0100)
servers/lloadd/client.c
servers/lloadd/connection.c
servers/lloadd/upstream.c

index 0b6515d5bf1d7e39260f1083dadb1df00bc51e7c..5f6d59cae675bbf4183ad3f95ebc13336e3573ef 100644 (file)
@@ -600,22 +600,29 @@ client_init(
     }
     c->c_write_event = event;
 
+    CONNECTION_LOCK(c);
+#ifdef BALANCER_MODULE
+    if ( lload_monitor_client_subsys ) {
+        acquire_ref( &c->c_refcnt );
+        CONNECTION_UNLOCK(c);
+        if ( lload_monitor_conn_entry_create(
+                    c, lload_monitor_client_subsys ) ) {
+            CONNECTION_LOCK(c);
+            RELEASE_REF( c, c_refcnt, c->c_destroy );
+            goto fail;
+        }
+        CONNECTION_LOCK(c);
+        RELEASE_REF( c, c_refcnt, c->c_destroy );
+    }
+#endif /* BALANCER_MODULE */
+
     c->c_destroy = client_destroy;
     c->c_unlink = client_unlink;
     c->c_pdu_cb = handle_one_request;
 
-    CONNECTION_LOCK(c);
     /* We only register the write event when we have data pending */
     event_add( c->c_read_event, c->c_read_timeout );
 
-#ifdef BALANCER_MODULE
-    if ( lload_monitor_client_subsys &&
-            lload_monitor_conn_entry_create(
-                    c, lload_monitor_client_subsys ) ) {
-        goto fail;
-    }
-#endif /* BALANCER_MODULE */
-
     checked_lock( &clients_mutex );
     LDAP_CIRCLEQ_INSERT_TAIL( &clients, c, c_next );
     checked_unlock( &clients_mutex );
@@ -623,6 +630,14 @@ client_init(
 
     return c;
 fail:
+    if ( !IS_ALIVE( c, c_live ) ) {
+        /*
+         * Released while we were unlocked, it's scheduled for destruction
+         * already
+         */
+        return NULL;
+    }
+
     if ( c->c_write_event ) {
         event_free( c->c_write_event );
         c->c_write_event = NULL;
index 73987341535549de46f18aa92a6dbb945f9440f5..fbb6492cef7254bf59e2018298fed33594b97b54 100644 (file)
@@ -522,7 +522,7 @@ lload_connection_close( LloadConnection *c, void *arg )
             "marking connection connid=%lu closing\n",
             c->c_connid );
 
-    /* We were approached from the connection list */
+    /* We were approached from the connection list or cn=monitor */
     assert( IS_ALIVE( c, c_refcnt ) );
 
     /* Need to acquire this first, even if we won't need it */
index e0329003fc79577385b8b07e74120b867c4f24e9..ef794b552574f7514c23825411e1ea73592200a4 100644 (file)
@@ -963,15 +963,26 @@ upstream_init( ber_socket_t s, LloadBackend *b )
     /* We only add the write event when we have data pending */
     c->c_write_event = event;
 
-    c->c_destroy = upstream_destroy;
-    c->c_unlink = upstream_unlink;
-
 #ifdef BALANCER_MODULE
-    if ( b->b_monitor && lload_monitor_conn_entry_create( c, b->b_monitor ) ) {
-        goto fail;
+    if ( b->b_monitor ) {
+        acquire_ref( &c->c_refcnt );
+        CONNECTION_UNLOCK(c);
+        checked_unlock( &b->b_mutex );
+        if ( lload_monitor_conn_entry_create( c, b->b_monitor ) ) {
+            RELEASE_REF( c, c_refcnt, c->c_destroy );
+            checked_lock( &b->b_mutex );
+            CONNECTION_LOCK(c);
+            goto fail;
+        }
+        checked_lock( &b->b_mutex );
+        CONNECTION_LOCK(c);
+        RELEASE_REF( c, c_refcnt, c->c_destroy );
     }
 #endif /* BALANCER_MODULE */
 
+    c->c_destroy = upstream_destroy;
+    c->c_unlink = upstream_unlink;
+
 #ifdef HAVE_TLS
     if ( c->c_is_tls == LLOAD_CLEARTEXT ) {
 #endif /* HAVE_TLS */
@@ -1015,6 +1026,14 @@ upstream_init( ber_socket_t s, LloadBackend *b )
     return c;
 
 fail:
+    if ( !IS_ALIVE( c, c_live ) ) {
+        /*
+         * Released while we were unlocked, it's scheduled for destruction
+         * already
+         */
+        return NULL;
+    }
+
     if ( c->c_write_event ) {
         event_del( c->c_write_event );
         event_free( c->c_write_event );