NACK from me.
cl_node and NCS_UNLOCK are not related at all, and freeing cl_node before
NCS_UNLOCK does not have any effect on NCS_UNLOCK.
There is a bug in locking/unlocking of the control block lock in #560 that I
missed in the review, and which may cause this assert if a client uses multiple
threads.
195 proc_rc = imma_startup(NCSMDS_SVC_ID_IMMA_OM);
196 if (NCSCC_RC_SUCCESS != proc_rc) {
197 TRACE_4("ERR_LIBRARY: imma startup failed:%u", proc_rc);
198 rc = SA_AIS_ERR_LIBRARY;
199 goto lock_fail;
200 }
201
202 if (false == cb->is_immnd_up) {
203 TRACE_2("ERR_TRY_AGAIN: IMMND is DOWN");
204 rc = SA_AIS_ERR_TRY_AGAIN;
205 goto lock_fail;
206 }
... and...
219 if (m_NCS_LOCK(&cb->cb_lock, NCS_LOCK_WRITE) !=
NCSCC_RC_SUCCESS) {
220 TRACE_4("ERR_LIBRARY: Lock failed");
221 rc = SA_AIS_ERR_LIBRARY;
222 goto lock_fail;
223 }
If any of these IF statements fails, the execution goes to lock_fail label,
where IMMA control block lock will be unlocked, even if it's not locked earlier
by the same thread. It may cause unwanted results in other threads.
Keep in mind that initially "locked" I set to true, and next code unlocks the
control block lock:
385 if (locked)
386 m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE);
Best regards,
Zoran
-----Original Message-----
From: [email protected] [mailto:[email protected]]
Sent: den 22 oktober 2013 15:41
To: Anders Björnerstedt
Cc: [email protected]
Subject: [devel] [PATCH 1 of 1] IMM: free the client node intialization of IMMA
fails(#602)
osaf/libs/agents/saf/imma/imma_om_api.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
If the intialization of IMMA agen fails, then free the cl_node.
diff --git a/osaf/libs/agents/saf/imma/imma_om_api.c
b/osaf/libs/agents/saf/imma/imma_om_api.c
--- a/osaf/libs/agents/saf/imma/imma_om_api.c
+++ b/osaf/libs/agents/saf/imma/imma_om_api.c
@@ -202,7 +202,7 @@ static SaAisErrorT initialize_common(SaI
if (false == cb->is_immnd_up) {
TRACE_2("ERR_TRY_AGAIN: IMMND is DOWN");
rc = SA_AIS_ERR_TRY_AGAIN;
- goto lock_fail;
+ goto end;
}
if((timeout_env_value = getenv("IMMA_SYNCR_TIMEOUT"))!=NULL) {
@@ -377,10 +377,6 @@ static SaAisErrorT initialize_common(SaI
ipc_init_fail:
lock_fail:
- if (rc != SA_AIS_OK) {
- free(cl_node);
- cl_node=NULL;
- }
if (locked)
m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE);
@@ -395,7 +391,7 @@ static SaAisErrorT initialize_common(SaI
/* Went well, return immHandle to the application */
*immHandle = cl_node->handle;
}
-
+ end:
if (rc != SA_AIS_OK) {
if (NCSCC_RC_SUCCESS != imma_shutdown(NCSMDS_SVC_ID_IMMA_OM)) {
/* Oh boy. Failure in imma_shutdown when we already have
@@ -404,6 +400,10 @@ static SaAisErrorT initialize_common(SaI
rc = SA_AIS_ERR_LIBRARY;
}
+ if (cl_node){
+ free(cl_node);
+ cl_node=NULL;
+ }
}
TRACE_LEAVE();
------------------------------------------------------------------------------
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
the latest Intel processors and coprocessors. See abstracts and register >
http://pubads.g.doubleclick.net/gampad/clk?id=60135991&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
the latest Intel processors and coprocessors. See abstracts and register >
http://pubads.g.doubleclick.net/gampad/clk?id=60135991&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
