protocol/server: Make conn object ref-counted

Change-Id: I992a7f8a75edfe7d75afaa1abe0ad45e8f351c8b
BUG: 796581
Signed-off-by: Pranith Kumar K <pranithk@gluster.com>
Reviewed-on: http://review.gluster.com/2806
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vijay@gluster.com>

5 files changed, 90 insertions, 198 deletions

diff --git a/xlators/protocol/server/src/server-handshake.c b/xlators/protocol/server/src/server-handshake.c
index 374f5a49a..d1e3659ab 100644
--- a/xlators/protocol/server/src/server-handshake.c
+++ b/xlators/protocol/server/src/server-handshake.c
@@ -356,6 +356,7 @@ server_setvolume (rpcsvc_request_t *req)
         int32_t              mgmt_version  = 0;
         uint32_t             lk_version    = 0;
         char                *buf           = NULL;
+        gf_boolean_t        cancelled      = _gf_false;
 
         params = dict_new ();
         reply  = dict_new ();
@@ -430,7 +431,9 @@ server_setvolume (rpcsvc_request_t *req)
                 goto fail;
         }
 
-        server_cancel_conn_timer (this, conn);
+        cancelled = server_cancel_conn_timer (this, conn);
+        if (cancelled)
+                server_conn_unref (conn);
         if (conn->lk_version != 0 &&
             conn->lk_version != lk_version) {
                 (void) server_connection_cleanup (this, conn);
@@ -720,7 +723,7 @@ server_set_lk_version (rpcsvc_request_t *req)
 
         conn = server_connection_get (this, args.uid);
         conn->lk_version = args.lk_ver;
-        server_connection_put (this, conn);
+        server_conn_unref (conn);
 
         rsp.lk_ver   = args.lk_ver;
 
diff --git a/xlators/protocol/server/src/server-helpers.c b/xlators/protocol/server/src/server-helpers.c
index cfa5ea46e..28d59aa44 100644
--- a/xlators/protocol/server/src/server-helpers.c
+++ b/xlators/protocol/server/src/server-helpers.c
@@ -257,6 +257,8 @@ server_nop_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
         GF_VALIDATE_OR_GOTO ("server", cookie, out);
         GF_VALIDATE_OR_GOTO ("server", this, out);
 
+        if (frame->root->trans)
+                server_conn_unref (frame->root->trans);
         state = CALL_STATE(frame);
 
         if (state)
@@ -298,6 +300,7 @@ do_lock_table_cleanup (xlator_t *this, server_connection_t *conn,
         }
         UNLOCK (&ltable->lock);
 
+        LOCK_DESTROY (&ltable->lock);
         GF_FREE (ltable);
 
         flock.l_type  = F_UNLCK;
@@ -314,7 +317,7 @@ do_lock_table_cleanup (xlator_t *this, server_connection_t *conn,
                   to release all locks from this transport
                 */
                 tmp_frame->root->pid         = 0;
-                tmp_frame->root->trans       = conn;
+                tmp_frame->root->trans       = server_conn_ref (conn);
                 memset (&tmp_frame->root->lk_owner, 0, sizeof (gf_lkowner_t));
 
                 if (locker->fd) {
@@ -361,7 +364,7 @@ do_lock_table_cleanup (xlator_t *this, server_connection_t *conn,
                 tmp_frame = copy_frame (frame);
 
                 tmp_frame->root->pid         = 0;
-                tmp_frame->root->trans       = conn;
+                tmp_frame->root->trans       = server_conn_ref (conn);
                 memset (&tmp_frame->root->lk_owner, 0, sizeof (gf_lkowner_t));
 
                 if (locker->fd) {
@@ -427,6 +430,8 @@ server_connection_cleanup_flush_cbk (call_frame_t *frame, void *cookie,
         fd_unref (fd);
         frame->local = NULL;
 
+        if (frame->root->trans)
+                server_conn_unref (frame->root->trans);
         STACK_DESTROY (frame->root);
 
         ret = 0;
@@ -478,7 +483,7 @@ do_fd_cleanup (xlator_t *this, server_connection_t *conn, call_frame_t *frame,
                         tmp_frame->local = fd;
 
                         tmp_frame->root->pid = 0;
-                        tmp_frame->root->trans = conn;
+                        tmp_frame->root->trans = server_conn_ref (conn);
                         memset (&tmp_frame->root->lk_owner, 0,
                                 sizeof (gf_lkowner_t));
 
@@ -571,20 +576,12 @@ out:
 int
 server_connection_destroy (xlator_t *this, server_connection_t *conn)
 {
-        call_frame_t       *frame = NULL, *tmp_frame = NULL;
         xlator_t           *bound_xl = NULL;
         int32_t             ret = -1;
-        server_state_t     *state = NULL;
         struct list_head    inodelk_lockers;
         struct list_head    entrylk_lockers;
         struct _lock_table *ltable = NULL;
-        struct _locker     *locker = NULL, *tmp = NULL;
-        struct gf_flock        flock = {0,};
-        fd_t               *fd = NULL;
-        int32_t             i = 0;
-        fdentry_t          *fdentries = NULL;
-        uint32_t             fd_count = 0;
-        char               *path      = NULL;
+        fdtable_t          *fdtable = NULL;
 
         GF_VALIDATE_OR_GOTO ("server", this, out);
         GF_VALIDATE_OR_GOTO ("server", conn, out);
@@ -592,17 +589,16 @@ server_connection_destroy (xlator_t *this, server_connection_t *conn)
         bound_xl = (xlator_t *) (conn->bound_xl);
 
         if (bound_xl) {
-                /* trans will have ref_count = 1 after this call, but its
-                   ok since this function is called in
-                   GF_EVENT_TRANSPORT_CLEANUP */
-                frame = create_frame (this, this->ctx->pool);
-
                 pthread_mutex_lock (&(conn->lock));
                 {
                         if (conn->ltable) {
                                 ltable = conn->ltable;
                                 conn->ltable = NULL;
                         }
+                        if (conn->fdtable) {
+                                fdtable = conn->fdtable;
+                                conn->fdtable = NULL;
+                        }
                 }
                 pthread_mutex_unlock (&conn->lock);
 
@@ -615,161 +611,69 @@ server_connection_destroy (xlator_t *this, server_connection_t *conn)
                                 list_splice_init (&ltable->inodelk_lockers,
                                                   &inodelk_lockers);
 
-                                list_splice_init (&ltable->entrylk_lockers, &entrylk_lockers);
+                                list_splice_init (&ltable->entrylk_lockers,
+                                                  &entrylk_lockers);
                         }
                         UNLOCK (&ltable->lock);
+                        LOCK_DESTROY (&ltable->lock);
                         GF_FREE (ltable);
                 }
 
-                flock.l_type  = F_UNLCK;
-                flock.l_start = 0;
-                flock.l_len   = 0;
-                list_for_each_entry_safe (locker,
-                                          tmp, &inodelk_lockers, lockers) {
-                        tmp_frame = copy_frame (frame);
-                        /*
-                          lock_owner = 0 is a special case that tells posix-locks
-                          to release all locks from this transport
-                        */
-                        tmp_frame->root->trans = conn;
-                        memset (&tmp_frame->root->lk_owner, 0,
-                                sizeof (gf_lkowner_t));
-
-                        if (locker->fd) {
-                                GF_ASSERT (locker->fd->inode);
-
-                                ret = inode_path (locker->fd->inode, NULL, &path);
-
-                                if (ret > 0) {
-                                        gf_log (this->name, GF_LOG_INFO, "finodelk "
-                                                "released on %s", path);
-                                        GF_FREE (path);
-                                } else {
+                GF_ASSERT (list_empty (&inodelk_lockers));
+                GF_ASSERT (list_empty (&entrylk_lockers));
 
-                                        gf_log (this->name, GF_LOG_INFO, "finodelk "
-                                                "released on inode with gfid %s",
-                                                uuid_utoa (locker->fd->inode->gfid));
-                                }
-
-                                STACK_WIND (tmp_frame, server_nop_cbk, bound_xl,
-                                            bound_xl->fops->finodelk,
-                                            locker->volume,
-                                            locker->fd, F_SETLK, &flock);
-                                fd_unref (locker->fd);
-                        } else {
-                                gf_log (this->name, GF_LOG_INFO, "inodelk "
-                                        "released on %s", locker->loc.path);
-
-                                STACK_WIND (tmp_frame, server_nop_cbk, bound_xl,
-                                            bound_xl->fops->inodelk,
-                                            locker->volume,
-                                            &(locker->loc), F_SETLK, &flock);
-                                loc_wipe (&locker->loc);
-                        }
-
-                        GF_FREE (locker->volume);
-
-                        list_del_init (&locker->lockers);
-                        GF_FREE (locker);
-                }
-
-                tmp = NULL;
-                locker = NULL;
-                list_for_each_entry_safe (locker, tmp, &entrylk_lockers, lockers) {
-                        tmp_frame = copy_frame (frame);
-
-                        tmp_frame->root->trans = conn;
-                        memset (&tmp_frame->root->lk_owner, 0,
-                                sizeof (gf_lkowner_t));
-
-                        if (locker->fd) {
-                                GF_ASSERT (locker->fd->inode);
-
-                                ret = inode_path (locker->fd->inode, NULL, &path);
-
-                                if (ret > 0) {
-                                        gf_log (this->name, GF_LOG_INFO, "fentrylk "
-                                                "released on %s", path);
-
-                                        GF_FREE (path);
-                                } else {
-
-                                        gf_log (this->name, GF_LOG_INFO, "fentrylk "
-                                                "released on inode with gfid %s",
-                                                uuid_utoa (locker->fd->inode->gfid));
-                                }
+                if (fdtable)
+                        gf_fd_fdtable_destroy (fdtable);
+        }
 
-                                STACK_WIND (tmp_frame, server_nop_cbk, bound_xl,
-                                            bound_xl->fops->fentrylk,
-                                            locker->volume,
-                                            locker->fd, NULL,
-                                            ENTRYLK_UNLOCK, ENTRYLK_WRLCK);
-                                fd_unref (locker->fd);
-                        } else {
-                                gf_log (this->name, GF_LOG_INFO, "entrylk "
-                                        "released on %s", locker->loc.path);
-
-                                STACK_WIND (tmp_frame, server_nop_cbk, bound_xl,
-                                            bound_xl->fops->entrylk,
-                                            locker->volume,
-                                            &(locker->loc), NULL,
-                                            ENTRYLK_UNLOCK, ENTRYLK_WRLCK);
-                                loc_wipe (&locker->loc);
-                        }
+        gf_log (this->name, GF_LOG_INFO, "destroyed connection of %s",
+                conn->id);
 
-                        GF_FREE (locker->volume);
+        pthread_mutex_destroy (&conn->lock);
+        GF_FREE (conn->id);
+        GF_FREE (conn);
+        ret = 0;
+out:
+        return ret;
+}
 
-                        list_del_init (&locker->lockers);
-                        GF_FREE (locker);
-                }
+server_connection_t*
+server_conn_unref (server_connection_t *conn)
+{
+        server_connection_t *todel = NULL;
+        xlator_t            *this = NULL;
 
-                pthread_mutex_lock (&(conn->lock));
-                {
-                        if (conn->fdtable) {
-                                fdentries = gf_fd_fdtable_get_all_fds (conn->fdtable,
-                                                                       &fd_count);
-                                gf_fd_fdtable_destroy (conn->fdtable);
-                                conn->fdtable = NULL;
-                        }
-                }
-                pthread_mutex_unlock (&conn->lock);
+        pthread_mutex_lock (&conn->lock);
+        {
+                conn->ref--;
 
-                if (fdentries != NULL) {
-                        for (i = 0; i < fd_count; i++) {
-                                fd = fdentries[i].fd;
-                                if (fd != NULL) {
-                                        tmp_frame = copy_frame (frame);
-                                        tmp_frame->local = fd;
-
-                                        STACK_WIND (tmp_frame,
-                                                    server_connection_cleanup_flush_cbk,
-                                                    bound_xl,
-                                                    bound_xl->fops->flush,
-                                                    fd);
-                                }
-                        }
-                        GF_FREE (fdentries);
+                if (!conn->ref) {
+                        list_del_init (&conn->list);
+                        todel = conn;
                 }
         }
+        pthread_mutex_unlock (&conn->lock);
 
-        if (frame) {
-                state = CALL_STATE (frame);
-                if (state)
-                        GF_FREE (state);
-                STACK_DESTROY (frame->root);
+        if (todel) {
+                this = THIS;
+                server_connection_destroy (this, todel);
+                conn = NULL;
         }
+        return conn;
+}
 
-        gf_log (this->name, GF_LOG_INFO, "destroyed connection of %s",
-                conn->id);
-
-        GF_FREE (conn->id);
-        GF_FREE (conn);
+server_connection_t*
+server_conn_ref (server_connection_t *conn)
+{
+        pthread_mutex_lock (&conn->lock);
+        {
+                conn->ref++;
+        }
+        pthread_mutex_unlock (&conn->lock);
 
-out:
-        return ret;
+        return conn;
 }
 
-
 server_connection_t *
 server_connection_get (xlator_t *this, const char *id)
 {
@@ -787,7 +691,6 @@ server_connection_get (xlator_t *this, const char *id)
                 list_for_each_entry (trav, &conf->conns, list) {
                         if (!strncmp (trav->id, id, strlen (id))) {
                                 conn = trav;
-                                conn->ref++;
                                 goto unlock;
                         }
                 }
@@ -800,7 +703,6 @@ server_connection_get (xlator_t *this, const char *id)
                 conn->id = gf_strdup (id);
                 /*'0' denotes uninitialised lock state*/
                 conn->lk_version = 0;
-                conn->ref++;
                 conn->fdtable = gf_fd_fdtable_alloc ();
                 conn->ltable  = gf_lock_table_new ();
                 conn->this    = this;
@@ -811,41 +713,12 @@ server_connection_get (xlator_t *this, const char *id)
 unlock:
         pthread_mutex_unlock (&conf->mutex);
 out:
+        if (conn)
+                server_conn_ref (conn);
         return conn;
 }
 
 
-void
-server_connection_put (xlator_t *this, server_connection_t *conn)
-{
-        server_conf_t       *conf = NULL;
-        server_connection_t *todel = NULL;
-
-        GF_VALIDATE_OR_GOTO ("server", this, out);
-        GF_VALIDATE_OR_GOTO ("server", conn, out);
-
-        conf = this->private;
-        GF_VALIDATE_OR_GOTO ("server", conf, out);
-
-        pthread_mutex_lock (&conf->mutex);
-        {
-                conn->ref--;
-
-                if (!conn->ref) {
-                        list_del_init (&conn->list);
-                        todel = conn;
-                }
-        }
-        pthread_mutex_unlock (&conf->mutex);
-
-        if (todel) {
-                server_connection_destroy (this, todel);
-        }
-
-out:
-        return;
-}
-
 static call_frame_t *
 server_alloc_frame (rpcsvc_request_t *req)
 {
@@ -907,7 +780,7 @@ get_frame_from_request (rpcsvc_request_t *req)
         frame->root->uid      = req->uid;
         frame->root->gid      = req->gid;
         frame->root->pid      = req->pid;
-        frame->root->trans    = req->trans->xl_private;
+        frame->root->trans    = server_conn_ref (req->trans->xl_private);
         frame->root->lk_owner = req->lk_owner;
 
         server_decode_groups (frame, req);
@@ -1521,13 +1394,16 @@ gf_server_check_setxattr_cmd (call_frame_t *frame, dict_t *dict)
         return 0;
 }
 
-void
+gf_boolean_t
 server_cancel_conn_timer (xlator_t *this, server_connection_t *conn)
 {
+        gf_timer_t      *timer = NULL;
+        gf_boolean_t    cancelled = _gf_false;
+
         if (!this || !conn) {
                 gf_log (THIS->name, GF_LOG_ERROR, "Invalid arguments to "
                         "cancel connection timer");
-                return;
+                return cancelled;
         }
 
         pthread_mutex_lock (&conn->lock);
@@ -1535,11 +1411,15 @@ server_cancel_conn_timer (xlator_t *this, server_connection_t *conn)
                 if (!conn->timer)
                         goto unlock;
 
-                gf_timer_call_cancel (this->ctx, conn->timer);
+                timer = conn->timer;
                 conn->timer = NULL;
         }
 unlock:
         pthread_mutex_unlock (&conn->lock);
 
-        return;
+        if (timer) {
+                gf_timer_call_cancel (this->ctx, timer);
+                cancelled = _gf_true;
+        }
+        return cancelled;
 }
diff --git a/xlators/protocol/server/src/server-helpers.h b/xlators/protocol/server/src/server-helpers.h
index 99ba7e546..9a502b478 100644
--- a/xlators/protocol/server/src/server-helpers.h
+++ b/xlators/protocol/server/src/server-helpers.h
@@ -68,7 +68,7 @@ server_print_request (call_frame_t *frame);
 call_frame_t *
 get_frame_from_request (rpcsvc_request_t *req);
 
-void
+gf_boolean_t
 server_cancel_conn_timer (xlator_t *this, server_connection_t *conn);
 
 void
diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c
index bfac42a27..7ac8590a4 100644
--- a/xlators/protocol/server/src/server.c
+++ b/xlators/protocol/server/src/server.c
@@ -41,6 +41,7 @@ grace_time_handler (void *data)
 {
         server_connection_t     *conn = NULL;
         xlator_t                *this = NULL;
+        gf_boolean_t            cancelled = _gf_false;
 
         conn = data;
         this = conn->this;
@@ -50,8 +51,11 @@ grace_time_handler (void *data)
 
         gf_log (this->name, GF_LOG_INFO, "grace timer expired");
 
-        server_cancel_conn_timer (this, conn);
-        server_connection_put (this, conn);
+        cancelled = server_cancel_conn_timer (this, conn);
+        if (cancelled) {
+                server_connection_cleanup (this, conn);
+                server_conn_unref (conn);
+        }
 out:
         return;
 }
@@ -172,6 +176,8 @@ ret:
         }
 
         if (frame) {
+                if (frame->root->trans)
+                        server_conn_unref (frame->root->trans);
                 STACK_DESTROY (frame->root);
         }
 
diff --git a/xlators/protocol/server/src/server.h b/xlators/protocol/server/src/server.h
index 4c38c1971..c54454e3e 100644
--- a/xlators/protocol/server/src/server.h
+++ b/xlators/protocol/server/src/server.h
@@ -75,8 +75,11 @@ typedef struct _server_connection server_connection_t;
 server_connection_t *
 server_connection_get (xlator_t *this, const char *id);
 
-void
-server_connection_put (xlator_t *this, server_connection_t *conn);
+server_connection_t*
+server_conn_unref (server_connection_t *conn);
+
+server_connection_t*
+server_conn_ref (server_connection_t *conn);
 
 int
 server_connection_cleanup (xlator_t *this, server_connection_t *conn);