lynxis lazus submitted this change.

View Change

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(-)

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 change 42133. To unsubscribe, or for help writing mail filters, visit settings.