Hi zoran,
I also just noticed these mis match. will resend the new patch for review.
/Neel.
On Wednesday 23 October 2013 07:33 PM, Zoran Milinkovic wrote:
> 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
