Avoiding msgb leaks is easiest if the caller retains ownership of the msgb. Take this hypothetical chain where leaks are obviously avoided:
void send() { msg = msgb_alloc(); dispatch(msg); msgb_free(msg); }
void dispatch(msg) { osmo_fsm_inst_dispatch(fi, msg); }
void fi_on_event(fi, data) { if (socket_is_ok) socket_write((struct msgb*)data); }
void socket_write(msgb) { if (!ok1) return; if (ok2) { if (!ok3) return; write(sock, msg->data); } }
However, if the caller passes ownership down to the msgb consumer, things become nightmarishly complex:
void send() { msg = msgb_alloc(); rc = dispatch(msg); /* dispatching event failed? */ if (rc) msgb_free(msg); }
int dispatch(msg) { if (osmo_fsm_inst_dispatch(fi, msg)) return -1; if (something_else()) return -1; // <-- double free! }
void fi_on_event(fi, data) { if (socket_is_ok) { socket_write((struct msgb*)data); else /* socket didn't consume? */ msgb_free(data); }
int socket_write(msgb) { if (!ok1) return -1; // <-- leak! if (ok2) { if (!ok3) goto out; write(sock, msg->data); } out: msgb_free(msg); return -2; }
If any link in this call chain fails to be aware of the importance to return a failed RC or to free a msgb if the chain is broken, we have a hidden msgb leak.
This is the case with osmo_sccp_user_sap_down(). In new osmo-msc, passing data through various FSM instances, there is high potential for leak/double-free bugs. A very large brain is required to track down every msgb path.
Isn't it possible to provide osmo_sccp_user_sap_down() in the caller-owns paradigm? Thinking about an osmo_sccp_user_sap_down2() that simply doesn't msgb_free().
Passing ownership to the consumer is imperative if a msg queue is involved that might send out asynchronously. (A workaround could be to copy the message before passing into the wqueue.) However, looks to me like no osmo_wqueue is involved in osmo_sccp_user_sap_down()? It already frees the msgb right upon returning, so this should be perfectly fine -- right?
I think I'll just try it, but if anyone knows a definite reason why this will not work, please let me know.
(Remotely related, I also still have this potential xua msg leak fix lying around, never got around to verify it: https://gerrit.osmocom.org/#/c/libosmo-sccp/+/9957/ )
~N