[prev in list] [next in list] [prev in thread] [next in thread] 

List:       fedora-directory-commits
Subject:    =?utf-8?q?=5B389-commits=5D?= [389-ds-base] 01/01: Ticket 48184 - clean up and delete connections at
From:       git () pagure ! io (git repository hosting)
Date:       2018-05-31 17:09:55
Message-ID: 20180531170954.718F92A965117 () pagure01 ! fedoraproject ! org
[Download RAW message or body]

This is an automated email from the git hooks/post-receive script.

tbordaz pushed a commit to branch 389-ds-base-1.3.8
in repository 389-ds-base.

commit 7ff19c6b2421293b7e1822b233d2cfa5f1a138cf
Author: Thierry Bordaz <tbordaz@redhat.com>
Date:   Wed May 30 15:48:11 2018 +0200

    Ticket 48184 - clean up and delete connections at shutdown (3rd)
    
    Bug description:
            During shutdown we would not close connections.
            In the past this may have just been an annoyance, but now with the way
            nunc-stans works, io events can still trigger on open xeisting \
connectinos  during shutdown.
    
    Fix Description:
            Because of NS dynamic it can happen that several jobs wants to work on \
                the
            same connection. In such case (a job is already set in c_job) we delay \
the  new job that will retry.
            In addition:
                - some call needed c_mutex
                - test uninitialized nunc-stans in case of shutdown while startup is \
not completed  
    	If it is not possible to schedule immediately a job it is sometime useless to \
                wait:
    		- if the connection is already freed, just cancel the scheduled job
    		  and do not register a new one
    		- If we are in middle of a shutdown we do not know if the
    		  scheduled job is ns_handle_closure, so cancel the scheduled
    		  job and schedule ns_handle_closure.
    
    https://pagure.io/389-ds-base/issue/48184
    
    Reviewed by:
                Original fix reviewed by Ludwig and Viktor
                Second   fix reviewed by Mark
    	    Third    fix reviewed by Mark
    
    Platforms tested: F26
    
    Flag Day: no
    
    Doc impact: no
---
 ldap/servers/slapd/connection.c | 10 ++++--
 ldap/servers/slapd/conntable.c  |  2 +-
 ldap/servers/slapd/daemon.c     | 67 +++++++++++++++++++++++++++++++----------
 ldap/servers/slapd/proto-slap.h |  2 +-
 4 files changed, 60 insertions(+), 21 deletions(-)

diff --git a/ldap/servers/slapd/connection.c b/ldap/servers/slapd/connection.c
index 76e8311..c54e7c2 100644
--- a/ldap/servers/slapd/connection.c
+++ b/ldap/servers/slapd/connection.c
@@ -741,14 +741,18 @@ connection_acquire_nolock(Connection *conn)
 
 /* returns non-0 if connection can be reused and 0 otherwise */
 int
-connection_is_free(Connection *conn)
+connection_is_free(Connection *conn, int use_lock)
 {
     int rc;
 
-    PR_EnterMonitor(conn->c_mutex);
+    if (use_lock) {
+        PR_EnterMonitor(conn->c_mutex);
+    }
     rc = conn->c_sd == SLAPD_INVALID_SOCKET && conn->c_refcnt == 0 &&
          !(conn->c_flags & CONN_FLAG_CLOSING);
-    PR_ExitMonitor(conn->c_mutex);
+    if (use_lock) {
+        PR_ExitMonitor(conn->c_mutex);
+    }
 
     return rc;
 }
diff --git a/ldap/servers/slapd/conntable.c b/ldap/servers/slapd/conntable.c
index f2f763d..114871d 100644
--- a/ldap/servers/slapd/conntable.c
+++ b/ldap/servers/slapd/conntable.c
@@ -129,7 +129,7 @@ connection_table_get_connection(Connection_Table *ct, int sd)
             break;
         }
 
-        if (connection_is_free(&(ct->c[index]))) {
+        if (connection_is_free(&(ct->c[index]), 1 /*use lock */)) {
             break;
         }
     }
diff --git a/ldap/servers/slapd/daemon.c b/ldap/servers/slapd/daemon.c
index 50e6747..35cfe7d 100644
--- a/ldap/servers/slapd/daemon.c
+++ b/ldap/servers/slapd/daemon.c
@@ -1699,7 +1699,8 @@ ns_connection_post_io_or_closing_try(Connection *conn)
     }
 
     /*
-     * Cancel any existing ns jobs we have registered.
+     * A job was already scheduled.
+     * Let it be dispatched first
      */
     if (conn->c_job != NULL) {
         return 1;
@@ -1780,25 +1781,59 @@ ns_connection_post_io_or_closing_try(Connection *conn)
     }
     return 0;
 }
+
+/*
+ * Tries to schedule I/O for this connection
+ * If the connection is already busy with a scheduled I/O
+ * it can wait until scheduled I/O is dispatched
+ *
+ * caller must hold c_mutex
+ */
 void
 ns_connection_post_io_or_closing(Connection *conn)
 {
     while (ns_connection_post_io_or_closing_try(conn)) {
-	/* we should retry later */
-	
-	/* We are not suppose to work immediately on the connection that is taken by
-	 * another job
-	 * release the lock and give some time
-	 */
-	
-	if (CONN_NEEDS_CLOSING(conn) && conn->c_ns_close_jobs) {
-	    return;
-	} else {
-	    PR_ExitMonitor(conn->c_mutex);
-	    DS_Sleep(PR_MillisecondsToInterval(100));
-
-	    PR_EnterMonitor(conn->c_mutex);
-	}
+        /* Here a job is currently scheduled (c->job is set) and not yet dispatched
+         * Job can be either:
+         *  - ns_handle_closure
+         *  - ns_handle_pr_read_ready
+         */
+
+        if (connection_is_free(conn, 0)) {
+            PRStatus shutdown_status;
+
+            /* The connection being freed,
+             * It means that ns_handle_closure already completed and the connection
+             * is no longer on the active list.
+             * The scheduled job is useless and scheduling a new one as well
+             */
+            shutdown_status = ns_job_done(conn->c_job);
+            if (shutdown_status != PR_SUCCESS) {
+                slapi_log_err(SLAPI_LOG_CRIT, "ns_connection_post_io_or_closing", \
"Failed cancel a job on a freed connection %d !\n", conn->c_sd); +            }
+            conn->c_job = NULL;
+            return;
+        }
+        if (g_get_shutdown() && CONN_NEEDS_CLOSING(conn)) {
+            PRStatus shutdown_status;
+
+            /* This is shutting down cancel any scheduled job to register \
ns_handle_closure +             */
+            shutdown_status = ns_job_done(conn->c_job);
+            if (shutdown_status != PR_SUCCESS) {
+                slapi_log_err(SLAPI_LOG_CRIT, "ns_connection_post_io_or_closing", \
"Failed to cancel a job during shutdown %d !\n", conn->c_sd); +            }
+            conn->c_job = NULL;
+            continue;
+        }
+
+        /* We are not suppose to work immediately on the connection that is taken by
+         * another job
+         * release the lock and give some time
+         */
+        PR_ExitMonitor(conn->c_mutex);
+        DS_Sleep(PR_MillisecondsToInterval(100));
+        PR_EnterMonitor(conn->c_mutex);
     }
 }
 
diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h
index b13334a..f54bc6b 100644
--- a/ldap/servers/slapd/proto-slap.h
+++ b/ldap/servers/slapd/proto-slap.h
@@ -1431,7 +1431,7 @@ int connection_acquire_nolock(Connection *conn);
 int connection_acquire_nolock_ext(Connection *conn, int allow_when_closing);
 int connection_release_nolock(Connection *conn);
 int connection_release_nolock_ext(Connection *conn, int release_only);
-int connection_is_free(Connection *conn);
+int connection_is_free(Connection *conn, int user_lock);
 int connection_is_active_nolock(Connection *conn);
 #if defined(USE_OPENLDAP)
 ber_slen_t openldap_read_function(Sockbuf_IO_Desc *sbiod, void *buf, ber_len_t len);

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.
_______________________________________________
389-commits mailing list -- 389-commits@lists.fedoraproject.org
To unsubscribe send an email to 389-commits-leave@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/389-commits@lists.fedoraproject.org/message/JJ7E7RXYXPPJRMCGYX3E52VHDX5VNY4H/



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic