This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
fixeria gerrit-no-reply at lists.osmocom.orgfixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/18881 ) Change subject: libosmo-mgcp-client: fix memleak in case if no response is received ...................................................................... Patch Set 5: (1 comment) This change is ready for review. https://gerrit.osmocom.org/c/osmo-mgw/+/18881/5/src/libosmo-mgcp-client/mgcp_client.c File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/18881/5/src/libosmo-mgcp-client/mgcp_client.c@1002 PS5, Line 1002: int mgcp_client_cancel(struct mgcp_client *mgcp, mgcp_trans_id_t trans_id) > it looks like osmo-bsc fails to call mgcp_client_cancel() in some situation, and the timeout or teardown should happen in osmo-bsc? As can be seen from the header file name (mgcp_client_internal.h), this is an internal API. Thus osmo-bsc is not supposed to call it. It's only used here, in this file, and by the FSM implementation (mgcp_client_fsm.c). > to identify the code path in osmo-bsc that might fail to call mgcp_client_cancel(). See above, osmo-bsc has no access to this API. > I'm particularly 'worried' about that comment saying "the FSM likely already terminated, thus we don't call osmo_fsm_inst_term()" Yes, the point is that a (potentially leaked) message is enqueued by an MGCP FSM instance _during termination_. Therefore nobody is responsible for tracking stalled messages in the queue. Normally the FSM itself handles response timeout and calls mgcp_client_cancel(). This is what I tried to explain in the commit message, perhaps I failed to make it easy to read/understand. > this patch is inviting trouble. This patch solves the problem. At least I don't see leaked chunks after each test case anymore. Yes, we can refactor everything, or simply do not enqueue such ownerless messages, but this would require more efforts to achieve the same goal - fix memleak. I am not against making this implementation better, feel free to do so if you want. But personally I don't want (nor can) spend even more time on this. It already took too much. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/18881 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ia2e89f31685a0822e5cb147a06cc1fc68efc1ec4 Gerrit-Change-Number: 18881 Gerrit-PatchSet: 5 Gerrit-Owner: fixeria <vyanitskiy at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier at sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de> Gerrit-Reviewer: laforge <laforge at osmocom.org> Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de> Gerrit-Comment-Date: Thu, 18 Jun 2020 22:35:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr at sysmocom.de> Gerrit-MessageType: comment -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200618/816d0957/attachment.htm>