[Openais] [PATCH]: openais/trunk: Remove lock in saLckResourceUnlockCallback

[Ryan O'Hara]

This patch changes the behavior of saLckResourceUnlock,
saLckResourceUnlockAsync, and saLckDispatch.

The old code would remove a lock as soon as either Unlock or
UnlockAsync received a result from the exec without error. The two
calls were exactly the same except that the async call generated a
callback.

This patch changes saLckResourceUnlockAsync such that the lock is not
removed from the database in the library until the callback it made.

This patch also changes both saLckResourceUnlock and
saLckResourceUnlockAsync such that if a lock is not found in the
executive layer, we do not return an error. it is valid for the
lock_id to exist in the library database but not the executive, as
this is a symptom of having an async unlock do the unlock but not
remove the lock from the database in the library because its callback
has not yet been processed.

Finally, saLckDispatch changes the way it deals with resource unlock
callbacks. If the lock_id specified in the callback exists, we remove
it. It it does not exist, it means that the lock was removed while the
callback was waiting to be processed. In this case we return
SA_AIS_ERR_NOT_EXIST, per the spec.

Ryan

-------------- next part --------------
Index: services/lck.c
===================================================================
--- services/lck.c	(revision 2002)
+++ services/lck.c	(working copy)
@@ -2499,15 +2499,21 @@
 		&req_exec_lck_resourceunlock->source,
 		req_exec_lck_resourceunlock->lock_id);
 
-	if (resource_lock == NULL) {
-		error = SA_AIS_ERR_NOT_EXIST;
-		goto error_exit;
+/* 	if (resource_lock == NULL) { */
+/* 		error = SA_AIS_ERR_NOT_EXIST; */
+/* 		goto error_exit; */
+/* 	} */
+
+/* 	lck_unlock (resource, resource_lock); */
+/* 	global_lock_count -= 1; */
+/* 	free (resource_lock); */
+
+	if (resource_lock != NULL) {
+		lck_unlock (resource, resource_lock);
+		global_lock_count -= 1;
+		free (resource_lock);
 	}
 
-	lck_unlock (resource, resource_lock);
-	global_lock_count -= 1;
-	free (resource_lock);
-
 error_exit:
 	if (api->ipc_source_is_local (&req_exec_lck_resourceunlock->source))
 	{
@@ -2551,15 +2557,21 @@
 		&req_exec_lck_resourceunlockasync->source,
 		req_exec_lck_resourceunlockasync->lock_id);
 
-	if (resource_lock == NULL) {
-		error = SA_AIS_ERR_NOT_EXIST;
-		goto error_exit;
+/* 	if (resource_lock == NULL) { */
+/* 		error = SA_AIS_ERR_NOT_EXIST; */
+/* 		goto error_exit; */
+/* 	} */
+
+/* 	lck_unlock (resource, resource_lock); */
+/* 	global_lock_count -= 1; */
+/* 	free (resource_lock); */
+
+	if (resource_lock != NULL) {
+		lck_unlock (resource, resource_lock);
+		global_lock_count -= 1;
+		free (resource_lock);
 	}
 
-	lck_unlock (resource, resource_lock);
-	global_lock_count -= 1;
-	free (resource_lock);
-
 error_exit:
 	if (api->ipc_source_is_local (&req_exec_lck_resourceunlockasync->source))
 	{
@@ -2582,6 +2594,8 @@
 
 		res_lib_lck_resourceunlock_callback.invocation =
 			req_exec_lck_resourceunlockasync->invocation;
+		res_lib_lck_resourceunlock_callback.lock_id =
+			req_exec_lck_resourceunlockasync->lock_id;
 
 		api->ipc_dispatch_send (
 			req_exec_lck_resourceunlockasync->source.conn,
Index: include/ipc_lck.h
===================================================================
--- include/ipc_lck.h	(revision 2002)
+++ include/ipc_lck.h	(working copy)
@@ -206,6 +206,7 @@
 struct res_lib_lck_resourceunlock_callback {
 	coroipc_response_header_t header __attribute__((aligned(8)));
 	mar_invocation_t invocation __attribute__((aligned(8)));
+	mar_uint64_t lock_id __attribute__((aligned(8)));
 } __attribute__((aligned(8)));
 
 #endif /* IPC_LCK_H_DEFINED */
Index: lib/lck.c
===================================================================
--- lib/lck.c	(revision 2002)
+++ lib/lck.c	(working copy)
@@ -410,10 +410,30 @@
 			res_lib_lck_resourceunlock_callback =
 				(struct res_lib_lck_resourceunlock_callback *)dispatch_data;
 
+			/*
+			 * Check that the lock_id is still valid before invoking
+			 * the callback. This is needed to handle the case where
+			 * a lock was unlocked before the callback was processed.
+			 */
+			error = hdb_error_to_sa (hdb_handle_get (&lckLockIdHandleDatabase,
+				res_lib_lck_resourceunlock_callback->lock_id,
+				(void *)&lckLockIdInstance));
+			if (error != SA_AIS_OK) {
+				res_lib_lck_resourceunlock_callback->header.error =
+					SA_AIS_ERR_NOT_EXIST;
+			}
+
 			callbacks.saLckResourceUnlockCallback (
 				res_lib_lck_resourceunlock_callback->invocation,
 				res_lib_lck_resourceunlock_callback->header.error);
 
+			if (error == SA_AIS_OK) {
+				hdb_handle_put (&lckLockIdHandleDatabase,
+					res_lib_lck_resourceunlock_callback->lock_id);
+
+				lckLockIdInstanceFinalize (lckLockIdInstance);
+			}
+
 			break;
 
 		default:
@@ -1122,6 +1142,7 @@
 	}
 
 	hdb_handle_put (&lckLockIdHandleDatabase, lockId);
+
 	lckLockIdInstanceFinalize (lckLockIdInstance);
 
 	return (error);
@@ -1215,8 +1236,9 @@
 	}
 
 	hdb_handle_put (&lckLockIdHandleDatabase, lockId);
-	lckLockIdInstanceFinalize (lckLockIdInstance);
 
+	/* lckLockIdInstanceFinalize (lckLockIdInstance); */
+
 	return (error);
 
 error_put: