Same patch but with a hdb_handle_put if hdb_handle_get returned OK.
Ryan
On Wed, Jun 10, 2009 at 11:57:31AM -0500, Ryan O'Hara wrote:
>
> If we request a lock via saLckLockRequestAsync, which is granted, an
> saLckLockGrantCallback will be generated. The callback request is sent
> to the dispatcher, but it is possible to unlock this lock before the
> callback is invoked. The specification clearly states that this should
> result in saLckLockGrantCallback returning SA_AIS_ERR_NOT_EXIST.
>
> The best way to handle this is to add a check to saLckDispatch that
> verifies that the lock_id is valid before invoking
> saLckLockGrantCallback. This add this check.
>
> Thanks again to Honzaf for finding this bug.
>
> Ryan
>
> Index: include/ipc_lck.h
> ===================================================================
> --- include/ipc_lck.h (revision 1951)
> +++ include/ipc_lck.h (working copy)
> @@ -192,6 +192,7 @@
> coroipc_response_header_t header __attribute__((aligned(8)));
> mar_invocation_t invocation __attribute__((aligned(8)));
> mar_uint32_t lock_status __attribute__((aligned(8)));
> + mar_uint64_t lock_id __attribute__((aligned(8)));
> } __attribute__((aligned(8)));
>
> struct res_lib_lck_lockwaiter_callback {
> Index: services/lck.c
> ===================================================================
> --- services/lck.c (revision 1951)
> +++ services/lck.c (working copy)
> @@ -1674,6 +1674,8 @@
> if (resource_lock != NULL) {
> res_lib_lck_lockgrant_callback.lock_status =
> resource_lock->lock_status;
> + res_lib_lck_lockgrant_callback.lock_id =
> + resource_lock->lock_id;
> res_lib_lck_lockgrant_callback.invocation =
> resource_lock->invocation;
> }
> Index: lib/lck.c
> ===================================================================
> --- lib/lck.c (revision 1951)
> +++ lib/lck.c (working copy)
> @@ -288,6 +288,7 @@
> SaDispatchFlagsT dispatchFlags)
> {
> struct lckInstance *lckInstance;
> + struct lckLockIdInstance *lckLockIdInstance;
> struct res_lib_lck_resourceopen_callback
> *res_lib_lck_resourceopen_callback;
> struct res_lib_lck_lockgrant_callback *res_lib_lck_lockgrant_callback;
> struct res_lib_lck_lockwaiter_callback *res_lib_lck_lockwaiter_callback;
> @@ -363,6 +364,19 @@
> res_lib_lck_lockgrant_callback =
> (struct res_lib_lck_lockgrant_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_lockgrant_callback->lock_id,
> + (void *)&lckLockIdInstance));
> + if (error != SA_AIS_OK) {
> + res_lib_lck_lockgrant_callback->header.error =
> + SA_AIS_ERR_NOT_EXIST;
> + }
> +
> callbacks.saLckLockGrantCallback (
> res_lib_lck_lockgrant_callback->invocation,
> res_lib_lck_lockgrant_callback->lock_status,
> _______________________________________________
> Openais mailing list
> [email protected]
> https://lists.linux-foundation.org/mailman/listinfo/openais
Index: include/ipc_lck.h
===================================================================
--- include/ipc_lck.h (revision 1951)
+++ include/ipc_lck.h (working copy)
@@ -192,6 +192,7 @@
coroipc_response_header_t header __attribute__((aligned(8)));
mar_invocation_t invocation __attribute__((aligned(8)));
mar_uint32_t lock_status __attribute__((aligned(8)));
+ mar_uint64_t lock_id __attribute__((aligned(8)));
} __attribute__((aligned(8)));
struct res_lib_lck_lockwaiter_callback {
Index: services/lck.c
===================================================================
--- services/lck.c (revision 1951)
+++ services/lck.c (working copy)
@@ -1674,6 +1674,8 @@
if (resource_lock != NULL) {
res_lib_lck_lockgrant_callback.lock_status =
resource_lock->lock_status;
+ res_lib_lck_lockgrant_callback.lock_id =
+ resource_lock->lock_id;
res_lib_lck_lockgrant_callback.invocation =
resource_lock->invocation;
}
Index: lib/lck.c
===================================================================
--- lib/lck.c (revision 1951)
+++ lib/lck.c (working copy)
@@ -288,6 +288,7 @@
SaDispatchFlagsT dispatchFlags)
{
struct lckInstance *lckInstance;
+ struct lckLockIdInstance *lckLockIdInstance;
struct res_lib_lck_resourceopen_callback
*res_lib_lck_resourceopen_callback;
struct res_lib_lck_lockgrant_callback *res_lib_lck_lockgrant_callback;
struct res_lib_lck_lockwaiter_callback *res_lib_lck_lockwaiter_callback;
@@ -363,6 +364,23 @@
res_lib_lck_lockgrant_callback =
(struct res_lib_lck_lockgrant_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_lockgrant_callback->lock_id,
+ (void *)&lckLockIdInstance));
+ if (error != SA_AIS_OK) {
+ res_lib_lck_lockgrant_callback->header.error =
+ SA_AIS_ERR_NOT_EXIST;
+ }
+ else {
+ hdb_handle_put (&lckLockIdHandleDatabase,
+
res_lib_lck_lockgrant_callback->lock_id);
+ }
+
callbacks.saLckLockGrantCallback (
res_lib_lck_lockgrant_callback->invocation,
res_lib_lck_lockgrant_callback->lock_status,
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
