Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic when publishing to a path with runOnDemand without any clients reading #2636

Closed
4 of 13 tasks
mrlt8 opened this issue Nov 3, 2023 · 6 comments · Fixed by #2637
Closed
4 of 13 tasks

Panic when publishing to a path with runOnDemand without any clients reading #2636

mrlt8 opened this issue Nov 3, 2023 · 6 comments · Fixed by #2637
Labels
bug Something isn't working rtmp

Comments

@mrlt8
Copy link
Contributor

mrlt8 commented Nov 3, 2023

Which version are you using?

v1.2.1

Which operating system are you using?

  • Linux amd64 standard
  • Linux amd64 Docker
  • Linux arm64 standard
  • Linux arm64 Docker
  • Linux arm7 standard
  • Linux arm7 Docker
  • Linux arm6 standard
  • Linux arm6 Docker
  • Windows amd64 standard
  • Windows amd64 Docker (WSL backend)
  • macOS amd64 standard
  • macOS amd64 Docker
  • Other (please describe)

Describe the issue

MTX panics if runOnDemand is set for a path and publish to the path before a client starts reading from that path.

This was not an issue in v1.2.0 which would correctly terminate the stream.

Describe how to replicate the issue

  1. set runOnDemand for a specific path
  2. start the server
  3. publish to path before the client starts reading

or

MTX_PATHS_MYPATH_RUNONDEMAND='echo hi' ./mediamtx   

Did you attach the server logs?

yes

INF [RTSP] [conn 127.0.0.1:54971] opened
INF [RTSP] [session 3487b1c0] created by 127.0.0.1:54971
INF [RTSP] [session 3487b1c0] is publishing to path 'mypath', 1 track (H264)
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x187c8a3]

goroutine 7 [running]:
github.com/bluenviron/mediamtx/internal/core.(*path).onDemandPublisherStop(0xc00009a1e0, {0x1ac6138, 0x14})
	/s/internal/core/path.go:869 +0x143
github.com/bluenviron/mediamtx/internal/core.(*path).doOnDemandPublisherCloseTimer(...)
	/s/internal/core/path.go:504
github.com/bluenviron/mediamtx/internal/core.(*path).runInner(0xc00009a1e0)
	/s/internal/core/path.go:405 +0x653
github.com/bluenviron/mediamtx/internal/core.(*path).run(0xc00009a1e0)
	/s/internal/core/path.go:334 +0x325
created by github.com/bluenviron/mediamtx/internal/core.newPath in goroutine 1
	/s/internal/core/path.go:284 +0x676

Did you attach a network dump?

no

@aler9 aler9 added bug Something isn't working rtmp labels Nov 3, 2023
@aler9
Copy link
Member

aler9 commented Nov 3, 2023

I also discovered that in previous versions, putting something into runOnDemand that is not a publisher, causes the publisher to get disconnected after some seconds. I'll see what i can do.

@mrlt8
Copy link
Contributor Author

mrlt8 commented Nov 4, 2023

Thanks for the quick fix @aler9!

I can confirm that it resolves the panic introduced in v1.2.1, however, it seems to have change the functionality from earlier versions. In v1.2.0 and earlier, MTX would correctly terminate the path/stream if runOnDemand was set and there weren't any clients reading, regardless of whether the runOnDemand command started the stream or not:

 INF MediaMTX v1.2.0
...
 INF [RTSP] [conn 127.0.0.1:57503] opened
 INF [RTSP] [session 4e0f6bd8] created by 127.0.0.1:57503
 INF [RTSP] [session 4e0f6bd8] is publishing to path 'mypath', 1 track (H264)
 INF [RTSP] [conn 127.0.0.1:57503] closed: terminated
 INF [RTSP] [session 4e0f6bd8] destroyed: terminated
...
 INF [RTSP] [conn 127.0.0.1:57788] opened
 INF [RTSP] [session 06cf6c42] created by 127.0.0.1:57788
 INF [RTSP] [session 06cf6c42] is publishing to path 'mypath', 1 track (H264)
 INF [RTSP] [session 6773ba70] created by 127.0.0.1:57787
 INF [RTSP] [session 6773ba70] is reading from path 'mypath', with UDP, 1 track (H264)
 INF [RTSP] [session 6773ba70] destroyed: torn down by 127.0.0.1:57787
 INF [RTSP] [conn 127.0.0.1:57787] closed: read tcp 127.0.0.1:8554->127.0.0.1:57787: read: connection reset by peer
 INF [path mypath] runOnDemand command stopped: not needed by anyone
 INF [RTSP] [conn 127.0.0.1:57788] closed: terminated
 INF [RTSP] [session 06cf6c42] destroyed: terminated

In the latest build, it seems to leave path open/streaming:

 INF MediaMTX v1.2.1-16-gfaf7218
...
 INF [RTSP] [conn 127.0.0.1:58247] opened
 INF [RTSP] [session bf4af9fd] created by 127.0.0.1:58247
 INF [RTSP] [session bf4af9fd] is publishing to path 'mypath', 1 track (H264)
 INF [RTSP] [session 968628dc] created by 127.0.0.1:58240
 INF [RTSP] [session 968628dc] is reading from path 'mypath', with UDP, 1 track (H264)
 INF [RTSP] [session 968628dc] destroyed: torn down by 127.0.0.1:58240
 INF [RTSP] [conn 127.0.0.1:58240] closed: read tcp 127.0.0.1:8554->127.0.0.1:58240: read: connection reset by peer
 INF [path mypath] runOnDemand command stopped: not needed by anyone

@aler9
Copy link
Member

aler9 commented Nov 4, 2023

In v1.2.0 and earlier, MTX would correctly terminate the path/stream if runOnDemand was set and there weren't any clients reading, regardless of whether the runOnDemand command started the stream or not

Yes, i noticed that, but that feature was never documented and was not intended to be into the server. If you want to disconnect the publisher when there are no clients connected anymore, put the publisher into runOnDemand, that is called correctly in your logs:

INF [path mypath] runOnDemand command stopped: not needed by anyone

Thus a publisher is closed correctly when there are no clients anymore, and when the publisher is inside runOnDemand.

I'll also add runOnUnDemand in order to allow calling a script when runOnDemand is stopped.

@mrlt8
Copy link
Contributor Author

mrlt8 commented Nov 4, 2023

That's what I was initially thinking of doing, however, runOnDemand command stopped does not get triggered until a client starts and stops reading.

Would it be possible to have an alternate option that mimics the previous functionality by terminating a specific path if there aren't any clients reading or the last client stops reading?

Copy link
Contributor

This issue is mentioned in release v1.3.0 🚀
Check out the entire changelog by clicking here

Copy link
Contributor

This issue is being locked automatically because it has been closed for more than 6 months.
Please open a new issue in case you encounter a similar problem.

@github-actions github-actions bot locked and limited conversation to collaborators May 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working rtmp
Projects
None yet
2 participants