lynxis lazus has submitted this change. ( 
https://gerrit.osmocom.org/c/osmo-remsim/+/42133?usp=email )

Change subject: rspro_server: fix releasing timed out clients
......................................................................

rspro_server: fix releasing timed out clients

Fix crash on reconnects of clients if they timed out.

The duplicate check will use conn->peer of the old peer,
but conn->peer is NULL because rspro_client_conn_destroy() never destroys
the connection if conn->peer is valid when rspro_client_conn_destroy() is 
called.

void rspro_client_conn_destroy(conn)
{
        if (conn->peer) {
                peer = conn->peer;
                conn->peer = NULL;
                osmo_stream_srv_destroy(peer); /* calls sock_closed_cb()
        }
        [..]
}

int sock_closed_cb(peer)
{
        [..]
        if (conn->peer) {
                        osmo_fsm_inst_dispatch(conn->fi, CLNTC_E_TCP_DOWN, 
NULL); /* calls in the end rspro_client_conn_destroy() */
        }
        return 0;
}

Re-organize the clean up:
* rspro_client_conn_destroy() will be only called by the FSM clean up
* closed_cb will inform the fi to clean up

Fixes: 8cfe1d808a57 ("Use new osmo_ipa_ka_fsm_inst APIs from libosmo-netif")
Related: OS#6957
Change-Id: I1f7faf5ffdd909362c492ab434b63fa7e79ada95
---
M src/server/rspro_server.c
1 file changed, 11 insertions(+), 8 deletions(-)

Approvals:
  osmith: Looks good to me, approved
  pespin: Looks good to me, but someone else must approve
  Jenkins Builder: Verified




diff --git a/src/server/rspro_server.c b/src/server/rspro_server.c
index 4e3666c..70d59c1 100644
--- a/src/server/rspro_server.c
+++ b/src/server/rspro_server.c
@@ -763,18 +763,21 @@
 {
        struct rspro_client_conn *conn = osmo_stream_srv_get_data(peer);
        OSMO_ASSERT(conn);
+
        osmo_stream_srv_set_data(peer, NULL);
        if (conn->ka_fi) {
                osmo_ipa_ka_fsm_stop(conn->ka_fi);
                osmo_ipa_ka_fsm_free(conn->ka_fi);
                conn->ka_fi = NULL;
        }
-       if (conn->fi) {
-               if (conn->peer) {
-                       conn->peer = NULL;
+
+       if (conn->peer) {
+               conn->peer = NULL;
+               if (conn->fi)
                        osmo_fsm_inst_dispatch(conn->fi, CLNTC_E_TCP_DOWN, 
NULL);
-               } /* else: rspro conn is already being destroyed, do nothing. */
        }
+       /* else: rspro conn is already being destroyed, do nothing. */
+
        return 0;
 }

@@ -920,13 +923,13 @@
 /* only to be used by the FSM cleanup. */
 static void rspro_client_conn_destroy(struct rspro_client_conn *conn)
 {
-       /* this will internally call closed_cb() which will dispatch a TCP_DOWN 
event */
+       /* only the FSM should call this in the clean up, when conn->fi is 
already NULL */
+       OSMO_ASSERT(conn->fi == NULL);
+
        if (conn->peer) {
                struct osmo_stream_srv *peer = conn->peer;
-               conn->peer = NULL;
                osmo_stream_srv_destroy(peer);
-               return;
-       } /* else: destroy initiated by conn->peer's closed_cb(). */
+       }

        /* ensure all slotmaps are unlinked + returned to NEW or deleted */
        slotmaps_wrlock(conn->srv->slotmaps);

--
To view, visit https://gerrit.osmocom.org/c/osmo-remsim/+/42133?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: merged
Gerrit-Project: osmo-remsim
Gerrit-Branch: master
Gerrit-Change-Id: I1f7faf5ffdd909362c492ab434b63fa7e79ada95
Gerrit-Change-Number: 42133
Gerrit-PatchSet: 2
Gerrit-Owner: lynxis lazus <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: lynxis lazus <[email protected]>
Gerrit-Reviewer: osmith <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>

Reply via email to