looks good
regards
-steve
On Thu, 2009-06-11 at 09:51 -0500, Ryan O'Hara wrote:
> 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
> _______________________________________________
> Openais mailing list
> [email protected]
> https://lists.linux-foundation.org/mailman/listinfo/openais
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
