Repository: bluenviron/gortsplib
Pull request #392 (2023-08-31 20:35:35)

server: fix crash when calling RECORD and PAUSE

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/bluenviron/gortsplib/pull/392?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron) Report
> Merging [#392](https://app.codecov.io/gh/bluenviron/gortsplib/pull/392?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron) (6e2cf0d) into [v4](https://app.codecov.io/gh/bluenviron/gortsplib/commit/fc0c24278931bd864c15b8187bb54d3a3d1486ad?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron) (fc0c242) will **increase** coverage by `0.04%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##               v4     #392      +/-   ##
==========================================
+ Coverage   83.95%   84.00%   +0.04%     
==========================================
  Files         100      100              
  Lines       10397    10398       +1     
==========================================
+ Hits         8729     8735       +6     
+ Misses       1282     1277       -5     
  Partials      386      386              
```


| [Files Changed](https://app.codecov.io/gh/bluenviron/gortsplib/pull/392?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron) | Coverage Δ | |
|---|---|---|
| [server\_session.go](https://app.codecov.io/gh/bluenviron/gortsplib/pull/392?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron#diff-c2VydmVyX3Nlc3Npb24uZ28=) | `78.45% <100.00%> (+0.53%)` | :arrow_up: |
| [server\_session\_format.go](https://app.codecov.io/gh/bluenviron/gortsplib/pull/392?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron#diff-c2VydmVyX3Nlc3Npb25fZm9ybWF0Lmdv) | `87.30% <100.00%> (+0.20%)` | :arrow_up: |

... and [1 file with indirect coverage changes](https://app.codecov.io/gh/bluenviron/gortsplib/pull/392/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron)

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bluenviron)


Patch:
diff --git a/server_session.go b/server_session.go
index 0d567068..308cee8f 100644
--- a/server_session.go
+++ b/server_session.go
@@ -336,17 +336,6 @@ func (ss *ServerSession) run() {
 
 	ss.ctxCancel()
 
-	if ss.setuppedStream != nil {
-		ss.setuppedStream.readerSetInactive(ss)
-		ss.setuppedStream.readerRemove(ss)
-	}
-
-	for _, sm := range ss.setuppedMedias {
-		sm.stop()
-	}
-
-	ss.writer.stop()
-
 	// close all associated connections, both UDP and TCP
 	// except for the ones that called TEARDOWN
 	// (that are detached from the session just after the request)
@@ -359,6 +348,17 @@ func (ss *ServerSession) run() {
 		sc.removeSession(ss)
 	}
 
+	if ss.setuppedStream != nil {
+		ss.setuppedStream.readerSetInactive(ss)
+		ss.setuppedStream.readerRemove(ss)
+	}
+
+	ss.writer.stop()
+
+	for _, sm := range ss.setuppedMedias {
+		sm.stop()
+	}
+
 	ss.s.closeSession(ss)
 
 	if h, ok := ss.s.Handler.(ServerHandlerOnSessionClose); ok {
@@ -929,8 +929,6 @@ func (ss *ServerSession) handleRequestInner(sc *ServerConn, req *base.Request) (
 			sm.start()
 		}
 
-		ss.setuppedStream.readerSetActive(ss)
-
 		switch *ss.setuppedTransport {
 		case TransportUDP:
 			ss.udpCheckStreamTimer = time.NewTimer(ss.s.checkStreamPeriod)
@@ -945,6 +943,8 @@ func (ss *ServerSession) handleRequestInner(sc *ServerConn, req *base.Request) (
 			// writer.start() is called by ServerConn after the response has been sent
 		}
 
+		ss.setuppedStream.readerSetActive(ss)
+
 		rtpInfo, ok := generateRTPInfo(
 			ss.s.timeNow(),
 			ss.setuppedMediasOrdered,
@@ -1053,12 +1053,12 @@ func (ss *ServerSession) handleRequestInner(sc *ServerConn, req *base.Request) (
 			return res, err
 		}
 
-		ss.writer.stop()
-
 		if ss.setuppedStream != nil {
 			ss.setuppedStream.readerSetInactive(ss)
 		}
 
+		ss.writer.stop()
+
 		for _, sm := range ss.setuppedMedias {
 			sm.stop()
 		}
diff --git a/server_session_format.go b/server_session_format.go
index 560f63d5..58251ad4 100644
--- a/server_session_format.go
+++ b/server_session_format.go
@@ -58,6 +58,7 @@ func (sf *serverSessionFormat) start() {
 func (sf *serverSessionFormat) stop() {
 	if sf.rtcpReceiver != nil {
 		sf.rtcpReceiver.Close()
+		sf.rtcpReceiver = nil
 	}
 }
 


Test patch:
diff --git a/server_record_test.go b/server_record_test.go
index 850c28e4..3b7701a4 100644
--- a/server_record_test.go
+++ b/server_record_test.go
@@ -1541,3 +1541,75 @@ func TestServerRecordPacketNTP(t *testing.T) {
 
 	<-recv
 }
+
+func TestServerRecordPausePause(t *testing.T) {
+	s := &Server{
+		Handler: &testServerHandler{
+			onAnnounce: func(ctx *ServerHandlerOnAnnounceCtx) (*base.Response, error) {
+				return &base.Response{
+					StatusCode: base.StatusOK,
+				}, nil
+			},
+			onSetup: func(ctx *ServerHandlerOnSetupCtx) (*base.Response, *ServerStream, error) {
+				return &base.Response{
+					StatusCode: base.StatusOK,
+				}, nil, nil
+			},
+			onRecord: func(ctx *ServerHandlerOnRecordCtx) (*base.Response, error) {
+				return &base.Response{
+					StatusCode: base.StatusOK,
+				}, nil
+			},
+			onPause: func(ctx *ServerHandlerOnPauseCtx) (*base.Response, error) {
+				return &base.Response{
+					StatusCode: base.StatusOK,
+				}, nil
+			},
+		},
+		RTSPAddress:    "localhost:8554",
+		UDPRTPAddress:  "127.0.0.1:8000",
+		UDPRTCPAddress: "127.0.0.1:8001",
+	}
+
+	err := s.Start()
+	require.NoError(t, err)
+	defer s.Close()
+
+	nconn, err := net.Dial("tcp", "localhost:8554")
+	require.NoError(t, err)
+	defer nconn.Close()
+	conn := conn.NewConn(nconn)
+
+	medias := []*description.Media{{
+		Type: description.MediaTypeApplication,
+		Formats: []format.Format{&format.Generic{
+			PayloadTyp: 97,
+			RTPMa:      "private/90000",
+		}},
+	}}
+
+	doAnnounce(t, conn, "rtsp://localhost:8554/teststream", medias)
+
+	inTH := &headers.Transport{
+		Delivery: func() *headers.TransportDelivery {
+			v := headers.TransportDeliveryUnicast
+			return &v
+		}(),
+		Mode: func() *headers.TransportMode {
+			v := headers.TransportModeRecord
+			return &v
+		}(),
+		Protocol:    headers.TransportProtocolUDP,
+		ClientPorts: &[2]int{35466, 35467},
+	}
+
+	res, _ := doSetup(t, conn, "rtsp://localhost:8554/teststream/"+medias[0].Control, inTH, "")
+
+	session := readSession(t, res)
+
+	doRecord(t, conn, "rtsp://localhost:8554/teststream", session)
+
+	doPause(t, conn, "rtsp://localhost:8554/teststream", session)
+
+	doPause(t, conn, "rtsp://localhost:8554/teststream", session)
+}