@vingarzan commented on this pull request.
In src/modules/cdp/peerstatemachine.c:
> + // 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:
⬇️ Suggested change- // Ensure proper locking order - lock_release(p->lock); - session = cdp_get_session(msg->sessionId->data); - lock_get(p->lock); + // 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, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.