@vingarzan commented on this pull request.
> + // Ensure proper locking order
+ lock_release(p->lock);
session = cdp_get_session(msg->sessionId->data);
+ lock_get(p->lock);
I think this actually adds more to the current issue, maybe just in the
reversed order of operations.
IMHO the proper fix is to avoid overlapping critical zones, not releasing
temporarily the locks. If you do this release here, to avoid memory corruption,
you should do something like this suggestion:
```suggestion
// Avoid overlapping locks
// TODO copy first some peer identifier, like the p->fqdn (deep!)
tmp_fqdn = ....;
lock_release(p->lock);
p = 0;
session = cdp_get_session(msg->sessionId->data);
p = get_peer_by_fqdn(tmp_fqdn);
if p == nil {
// oh-oh, the peer is actually gone, so ... how do we resume and
unfold this now??
...
}
```
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/4191#pullrequestreview-2727970913
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/4191/review/2727970...@github.com>_______________________________________________
Kamailio - Development Mailing List -- sr-dev@lists.kamailio.org
To unsubscribe send an email to sr-dev-le...@lists.kamailio.org
Important: keep the mailing list in the recipients, do not reply only to the
sender!
