** CID 477972: Concurrent data access violations (MISSING_LOCK)
Closed this issue · 5 comments
alfredh commented
New coverity scan warnings:
Hi,
Please find the latest report on new defect(s) introduced to alfredh/re found with Coverity Scan.
2 new defect(s) introduced to alfredh/re found with Coverity Scan.
2 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 2 of 2 defect(s)
** CID 477972: Concurrent data access violations (MISSING_LOCK)
/src/rtp/sess.c: 470 in sdes_encode_handler()
________________________________________________________________________________________________________
*** CID 477972: Concurrent data access violations (MISSING_LOCK)
/src/rtp/sess.c: 470 in sdes_encode_handler()
464
465
466 static int sdes_encode_handler(struct mbuf *mb, void *arg)
467 {
468 struct rtcp_sess *sess = arg;
469
>>> CID 477972: Concurrent data access violations (MISSING_LOCK)
>>> Accessing "sess->cname" without holding lock "rtcp_sess.lock". Elsewhere, "rtcp_sess.cname" is written to with "rtcp_sess.lock" held 2 out of 4 times (1 of these accesses strongly imply that it is necessary).
470 return rtcp_sdes_encode(mb, rtp_sess_ssrc(sess->rs), 1,
471 RTCP_SDES_CNAME, sess->cname);
472 }
473
474
475 static int mk_sdes(struct rtcp_sess *sess, struct mbuf *mb)
** CID 477971: Concurrent data access violations (MISSING_LOCK)
/src/rtp/sess.c: 80 in sess_destructor()
________________________________________________________________________________________________________
*** CID 477971: Concurrent data access violations (MISSING_LOCK)
/src/rtp/sess.c: 80 in sess_destructor()
74
75 if (sess->cname)
76 (void)send_bye_packet(sess);
77
78 tmr_cancel(&sess->tmr);
79
>>> CID 477971: Concurrent data access violations (MISSING_LOCK)
>>> Accessing "sess->cname" without holding lock "rtcp_sess.lock". Elsewhere, "rtcp_sess.cname" is written to with "rtcp_sess.lock" held 2 out of 4 times (1 of these accesses strongly imply that it is necessary).
80 mem_deref(sess->cname);
81 hash_flush(sess->members);
82 mem_deref(sess->members);
83 mem_deref(sess->lock);
84 }
85
cspiel1 commented
I will have a look on this.
sreimers commented
Do we have to start the coverity workflow manually?
Yes only the coverity branch and a release is checked, because coverity has daily and weekly limits how often it can be run. So I merge manually from time to time.
sreimers commented
cspiel1 commented
The warning is very concrete. But it makes not much sense to lock the mem_deref(sess->cname)
just before the mutex and the whole sess
object is destroyed. The application has to ensure that the object is not destroyed before all other threads are finished. Do you think that a mutex lock in the destructor helps?