Yet another small change. Move the hdb_handle_put below the callback.
On Thu, Jun 11, 2009 at 09:24:31AM -0500, Ryan O'Hara wrote:
>
> 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
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,11 +364,29 @@
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,
res_lib_lck_lockgrant_callback->header.error);
+ if (error == SA_AIS_OK) {
+ hdb_handle_put (&lckLockIdHandleDatabase,
+
res_lib_lck_lockgrant_callback->lock_id);
+ }
+
break;
case MESSAGE_RES_LCK_LOCKWAITER_CALLBACK:
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
