From 240cfa58c62571b92640a385cfcce6d858cb00dc Mon Sep 17 00:00:00 2001

From: Thierry Bordaz <tbordaz@redhat.com>

Date: Wed, 30 May 2018 15:48:11 +0200

Subject: [PATCH] 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 76e83112b..c54e7c26c 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 f2f763dfa..114871d17 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 50e67474e..35cfe7de0 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 b13334ad1..f54bc6bc5 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);

-- 

2.17.0