osaf/libs/agents/saf/ntfa/ntfa_api.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
In a multithreaded application,saNtfFinalize() may hang forever
leading to deadlock, if one thread tries to call finalize the handle,
when other thread is trying to send a notification.
The reason is saNtfNotificationSend() API takes mutex_lock() on ntfa_cb three
times:
-In the first lock/unlock,it retrieves client record.
-In the second lock/unlock, it verifies notification parameters.
-In the third lock/unlock, it sets notificationId that it has got
from ntfs.
In saNtfFinalize() API also after receving response from the NTFS, it
tries to free all records for this client and also it releases allocated
memory for the notifications.
If saNtfFinalize() is called for the same client from one thread for which
Send() is in
progress in another thread, saNtfFinalize() may get lock on ntfa_cb and it will
try to
destroy all the records for this client. But here it will not be able to destroy
the record as Send() API is still using records in the lock free region.
Thus after taking a lock on ntfa_cb, saNtfFinalize() blocks itself for the
client
record to be released by Send(). Since Send() is in progress, in between it
will also
try to again take lock on ntfa_cb, but it will not get as it already owned by
saNtfFinalize()
which is already blocked because of client resources owned by Send().
As a solution, multiple locks should be avoided in the Send() API. Only while
retrieving
client record and notificationHanndle record it should use lock on ntfa_cb.
The reason is saNtfFinalize() will anyway wait for the resoureces taken by
Send() to
be released and thus it will not free the memory. In the worst case,
saNtfFinalize()
will complete in NTFS_WAIT_TIME, but there will not be any deadlock situation.
diff --git a/osaf/libs/agents/saf/ntfa/ntfa_api.c
b/osaf/libs/agents/saf/ntfa/ntfa_api.c
--- a/osaf/libs/agents/saf/ntfa/ntfa_api.c
+++ b/osaf/libs/agents/saf/ntfa/ntfa_api.c
@@ -1398,23 +1398,25 @@ SaAisErrorT saNtfNotificationSend(SaNtfN
goto done;
}
+ osafassert(pthread_mutex_lock(&ntfa_cb.cb_lock) == 0);
notification_hdl_rec = ncshm_take_hdl(NCS_SERVICE_ID_NTFA,
notificationHandle);
if (notification_hdl_rec == NULL) {
+ osafassert(pthread_mutex_unlock(&ntfa_cb.cb_lock) == 0);
TRACE("ncshm_take_hdl notificationHandle failed");
rc = SA_AIS_ERR_BAD_HANDLE;
goto err_free;
}
notification_hdl_rec->is_longdn_agent_owner = false;
- osafassert(pthread_mutex_lock(&ntfa_cb.cb_lock) == 0);
client_handle = notification_hdl_rec->parent_hdl->local_hdl;
- osafassert(pthread_mutex_unlock(&ntfa_cb.cb_lock) == 0);
/* retrieve client hdl rec */
client_rec = ncshm_take_hdl(NCS_SERVICE_ID_NTFA, client_handle);
if (client_rec == NULL) {
+ osafassert(pthread_mutex_unlock(&ntfa_cb.cb_lock) == 0);
TRACE("ncshm_take_hdl client_handle failed");
rc = SA_AIS_ERR_BAD_HANDLE;
goto done_give_hdl;
}
+ osafassert(pthread_mutex_unlock(&ntfa_cb.cb_lock) == 0);
/**
** Populate a sync MDS message
@@ -1427,7 +1429,6 @@ SaAisErrorT saNtfNotificationSend(SaNtfN
send_param->notificationType =
notification_hdl_rec->ntfNotificationType;
ntfsv_v_data_cp(&send_param->variable_data,
¬ification_hdl_rec->variable_data);
- osafassert(pthread_mutex_lock(&ntfa_cb.cb_lock) == 0);
/* Check parameters, depending on type */
switch (notification_hdl_rec->ntfNotificationType) {
case SA_NTF_TYPE_ALARM:
@@ -1486,7 +1487,6 @@ SaAisErrorT saNtfNotificationSend(SaNtfN
goto done_give_hdls;
}
- osafassert(pthread_mutex_unlock(&ntfa_cb.cb_lock) == 0);
/* Send a sync MDS message to obtain a notification id */
mds_rc = ntfa_mds_msg_sync_send(&ntfa_cb, &msg, &o_msg, timeout);
switch (mds_rc) {
@@ -1506,7 +1506,6 @@ SaAisErrorT saNtfNotificationSend(SaNtfN
TRACE("ntfa_mds_msg_sync_send FAILED: %u", rc);
rc = SA_AIS_ERR_TRY_AGAIN;
}
- osafassert(pthread_mutex_lock(&ntfa_cb.cb_lock) == 0);
if (mds_rc != NCSCC_RC_SUCCESS) {
goto done_give_hdls;
}
@@ -1556,7 +1555,6 @@ SaAisErrorT saNtfNotificationSend(SaNtfN
done_give_hdls:
if (o_msg)
ntfa_msg_destroy(o_msg);
- osafassert(pthread_mutex_unlock(&ntfa_cb.cb_lock) == 0);
ncshm_give_hdl(client_handle);
done_give_hdl:
ncshm_give_hdl(notificationHandle);
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
