Prevent xprt refcount going up from zero.

svc_xprt_lookup() looks up in hash table, places a ref and passes to the
caller. The xprt gets an additional initial refcount for being in the
hash table itself. The caller will do SVC_RELEASE() once he is done with
the xprt. SVC_DESTROY() gets called when the xprt gets an error which
will eventually make the additional hash refcount go away, finally
leading to zero refcount. The xprt is deleted only at the very end,
before destroying. Once an SVC_DESTROY() is called, the refcount could
go to zero, xprt gets deleted from hash table, then it gets destroyed.

svc_xprt_lookup() can fairly assume that xprt memory won't be freed as
long as it is in the hash table and it holds the hash lock. It should
NOT access the xprt after releasing the hash lock if the refcount was
bumped from zero. In such a case, we bailout and return failure.

TODO: If we can delete the xprt from the hash table (RBT) as part of
SVC_DESTROY() before decrementing the refcount, we will never have zero
refcount xprt in the hash table, and this special code can be avoided.

Author: malahal

src/svc_xprt.c

@@ -143,6 +143,7 @@ svc_xprt_lookup(int fd, svc_xprt_setup_t setup)
 	struct opr_rbtree_node *nv;
 	SVCXPRT *xprt = NULL;
 	uint16_t xp_flags;
+	int32_t refcount;
 
 	if (svc_xprt_init_failure())
 		return (NULL);
@@ -197,8 +198,26 @@ svc_xprt_lookup(int fd, svc_xprt_setup_t setup)
 	rec = opr_containerof(nv, struct rpc_dplx_rec, fd_node);
 	xprt = &rec->xprt;
 
-	/* lookup reference before unlock ensures shutdown cannot release */
-	SVC_REF(xprt, SVC_REF_FLAG_NONE);
+	/* lookup reference before unlock ensures shutdown cannot release
+	 *
+	 * The current implementation makes it possible to have 0
+	 * refcount xprts in the tree. The code would be easier if
+	 * destroy removes from the tree before decrementing the
+	 * refcount avoiding this possibility. It needs some code
+	 * re-org but easier to implement in the long run!
+	 *
+	 * For now, manually increment the refcnt and see if it indeed
+	 * was zero (1 now!). If so, bailout under tree lock as the xprt
+	 * might be freed as soon as we unlock!
+	 *
+	 * TODO: Need LTTng trace messages for these manual refcounts
+	 */
+	refcount = atomic_inc_int32_t(&xprt->xp_refcnt);
+	if (refcount == 1) { /* proces of getting destroyed */
+		atomic_dec_int32_t(&xprt->xp_refcnt);
+		rwlock_unlock(&t->lock);
+		return NULL;
+	}
 	rwlock_unlock(&t->lock);
 
 	/* unlocked window here permits shutdown to destroy without release;