search

meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8k stars 2.45k forks source link

Ports used by RTP Forward (videoroom plugin) do not get released #1605

Closed benjdavenport closed 5 years ago

benjdavenport commented 5 years ago

hey guys!

It appears that ports used by rtp_forward in the videoroom plugin do not get released. Destroying the room, handle, session, etc have no effect.

Observed on both Ubuntu 16.04 and Ubuntu 18.04.

I'll add more findings here as I come across them. :o)

lminiero commented 5 years ago

What do you mean by "not released"? RTP forward doesn't actually bind to any port itself, it connects to remote ports (and if those remain open that's normal, especially if allocated by the Streaming plugin for instance) and so the local bind is just random. We definitely close the sockets when getting rid of RTP forwarders, or at least should.

benjdavenport commented 5 years ago

What's observed is that when specifying ports for video/audio rtcp for a forwarder, the ports remain in a listening state and are unavailable for use even after tearing everything down. Only thing that works is restarting Janus. Per the other recently closed issue I reported it may be user error here as we had the flow direction backwards for forwarder rtcp, so it's possible this may have prevented things from being closed down properly. Just a hunch, I'll update here today.

On Wed, May 1, 2019 at 9:46 AM Lorenzo Miniero notifications@github.com wrote:

What do you mean by "not released"? RTP forward doesn't actually bind to any port itself, it connects to remote ports (and if those remain open that's normal, especially if allocated by the Streaming plugin for instance) and so the local bind is just random. We definitely close the sockets when getting rid of RTP forwarders, or at least should.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/meetecho/janus-gateway/issues/1605#issuecomment-488286229, or mute the thread https://github.com/notifications/unsubscribe-auth/ABSYVTP6SSXILM2V2E2NL4TPTGNLLANCNFSM4HJFYWRA .

lminiero commented 5 years ago

Mh the close for the rtcp_fd socket is definitely there when you destroy an RTP forwarder, so not sure what may be going wrong, if anything. If you can provide more feedback that would help, thanks!

benjdavenport commented 5 years ago

Unfortunately applying the rtp forwarder port to a sink (instead of a source) as per my misunderstanding earlier didn't have any affect on port closure. The ports used by the forwarder are still listening.

What specifically could I provide that may help shed light on the issue?

lminiero commented 5 years ago

Not sure why you think those ports are the ones used by the forwarders? Have you checked if they're just slow to be released, rather than never released at all?

benjdavenport commented 5 years ago

Hey Lorenzo,

Not sure why you think those ports are the ones used by the forwarders?

Before we send the command to rtp_forward we first request a list of the number of available ports we need, so we verified that the ports that are used for RTP forward are in fact still open.

Have you checked if they're just slow to be released, rather than never

released at all?

Yes, we have to restart Janus every few days to release the ports.

I just got back from Rome, so I'll dig into this more today and next week, I'll be sure to update here. :o)

On Thu, May 16, 2019 at 4:54 AM Lorenzo Miniero notifications@github.com wrote:

Not sure why you think those ports are the ones used by the forwarders? Have you checked if they're just slow to be released, rather than never released at all?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/meetecho/janus-gateway/issues/1605?email_source=notifications&email_token=ABSYVTO4BK5YHLRHKJ6SHRDPVUONTA5CNFSM4HJFYWRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVREPGQ#issuecomment-492980122, or mute the thread https://github.com/notifications/unsubscribe-auth/ABSYVTMRZBEY6HHNFMYJBQDPVUONTANCNFSM4HJFYWRA .

lminiero commented 5 years ago

I just got back from Rome

WHAT, you came to Rome and didn't pass by Napoli and say hello? :smile:

Thanks for the feedback though, I'll try to have a look next week. I know we are doing the proper cleanup when a forwarder is destroyed, that is closing the sockets, etc., so not sure what may be going wrong. If you can add some checks yourself as well that might expedite fixing the issue.

benjdavenport commented 5 years ago

Believe me I wanted nothing more than to come visit you guys and touch that glorious head of hair of yours for good luck. :o)

You got it though, I'll be as diligent as possible and dig as deep as I can. Thanks dude!

On Fri, May 17, 2019 at 11:16 AM Lorenzo Miniero notifications@github.com wrote:

I just got back from Rome

WHAT, you came to Rome and didn't pass by Napoli and say hello? 😄

Thanks for the feedback though, I'll try to have a look next week. I know we are doing the proper cleanup when a forwarder is destroyed, that is closing the sockets, etc., so not sure what may be going wrong. If you can add some checks yourself as well that might expedite fixing the issue.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/meetecho/janus-gateway/issues/1605?email_source=notifications&email_token=ABSYVTKKHF62MBQLKJRZQOLPV3D3DA5CNFSM4HJFYWRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVVBILI#issuecomment-493491245, or mute the thread https://github.com/notifications/unsubscribe-auth/ABSYVTNGGEG5MDB5VN6SRY3PV3D3DANCNFSM4HJFYWRA .

atuk721 commented 5 years ago

Hello.

I'm having the same problem. I'll show you my situation too.

I think only video RTCP port of forwarding ports is closed. I also have set audio RTCP port, but I think it is not used.

I'm glad this information is useful.

Environment

OS: Ubuntu 16.04.5 LTS (Xenial Xerus) Janus: v0.6.3 / v0.7.1

Situation

First of all

root@test:~# netstat -anp | grep janus
tcp6       0      0 :::8188                 :::*                    LISTEN      3720/janus

After WebRTC publishing

root@test:~# netstat -anp | grep janus
tcp6       0      0 :::8188                 :::*                    LISTEN      3720/janus
tcp6       0      0 192.168.2.1:8188      192.168.1.1:49390         ESTABLISHED 3720/janus
udp   129024      0 192.168.2.1:11947     0.0.0.0:*                             3720/janus

After rtp_forward

root@test:~# netstat -anp | grep janus
tcp6       0      0 :::8188                 :::*                    LISTEN      3720/janus
tcp6       0      0 192.168.2.1:8188        192.168.1.1:49390       ESTABLISHED 3720/janus
udp   153600      0 192.168.2.1:11947       0.0.0.0:*                           3720/janus
udp        0      0 0.0.0.0:23317           0.0.0.0:*                           3720/janus
udp        0      0 0.0.0.0:29792           0.0.0.0:*                           3720/janus
udp        0      0 0.0.0.0:62566           0.0.0.0:*                           3720/janus
Janus messages.

{"janus":"message","transaction":"971b2f6b-8669-418e-9340-d17ced31e8ba","body":{"request":"rtp_forward","room":6089667165809135,"publisher_id":4695723699567663,"host":"192.168.2.2","video_port":11862,"video_rtcp_port":11863,"video_pt":96,"audio_port":11864,"audio_rtcp_port":11865,"audio_pt":97},"handle_id":6536407549713940,"session_id":2780679814728687}

{"janus":"success","session_id":2780679814728687,"transaction":"971b2f6b-8669-418e-9340-d17ced31e8ba","sender":6536407549713940,"plugindata":{"plugin":"janus.plugin.videoroom","data":{"publisher_id":4695723699567663,"rtp_stream":{"audio_stream_id":175715319,"audio":11864,"video_stream_id":2982485463,"video":11862,"host":"192.168.2.2"}

After stop_rtp_forward, destroy room, close session

root@test:~# netstat -anp | grep janus
tcp6       0      0 :::8188                 :::*                    LISTEN      3720/janus
udp        0      0 0.0.0.0:29792           0.0.0.0:*                           3720/janus
udp        0      0 0.0.0.0:62566           0.0.0.0:*                           3720/janus
Janus messages.

{"janus":"message","transaction":"f1ab22b6-ebfc-4a6b-93af-25f7e99ffa28","body":{"request":"stop_rtp_forward","room":6089667165809135,"publisher_id":4695723699567663,"stream_id":2982485463},"handle_id":6536407549713940,"session_id":2780679814728687}
{"janus":"message","transaction":"803964bc-2d06-41f0-ac23-93a321dfdc88","body":{"request":"stop_rtp_forward","room":6089667165809135,"publisher_id":4695723699567663,"stream_id":175715319},"handle_id":6536407549713940,"session_id":2780679814728687}

{"janus":"success","session_id":2780679814728687,"transaction":"f1ab22b6-ebfc-4a6b-93af-25f7e99ffa28","sender":6536407549713940,"plugindata":{"plugin":"janus.plugin.videoroom","data":{"videoroom":"stop_rtp_forward","room":6089667165809135,"publisher_id":4695723699567663,"stream_id":2982485463}}}
{"janus":"success","session_id":2780679814728687,"transaction":"803964bc-2d06-41f0-ac23-93a321dfdc88","sender":6536407549713940,"plugindata":{"plugin":"janus.plugin.videoroom","data":{"videoroom":"stop_rtp_forward","room":6089667165809135,"publisher_id":4695723699567663,"stream_id":175715319}}}
lminiero commented 5 years ago

The ports that remain open don't appear neither in the rtp_forward request nor the response, though (the ports you bound to for audio and video are different), so I don't think it's ports allocated there. Unless for some reason we're allocating ports for simulcasting as well even though they're not used?

Edit: @alexamirante made me realize I'm an idiot :smile: Those are the target ports, not the ports we bind to... I was thinking of the Streaming plugin for some reason. I'll have a look to see what's not being released, if anything.

lminiero commented 5 years ago

Made some more tests and the cause is actually a reference counter leak. The RTP forwarder is never freed when RTCP is used (a reference remains there), and so we never get to the point where we close the socket. Investigating why it's happening now, so that I can work on a fix later.

lminiero commented 5 years ago

Fixed in the above commit, please let me know if you can confirm.

atuk721 commented 5 years ago

@lminiero I have tried latest commit and I have confirmed the problem is fixed. Thank you very much!

benjdavenport commented 5 years ago

@lminiero https://github.com/lminiero

Confirmed fixed with 947bfb0 https://github.com/meetecho/janus-gateway/commit/947bfb0ae5d5c1e29224c71d631607b6de1869c5 .

I missed this when looking for forward ref counters as the one that was already in there threw me off. :o) Thanks dude!

On Tue, May 28, 2019 at 6:15 AM Atsushi Koge notifications@github.com wrote:

@lminiero https://github.com/lminiero I have tried latest commit and I have confirmed the problem is fixed. Thank you very much!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/meetecho/janus-gateway/issues/1605?email_source=notifications&email_token=ABSYVTOIR4Z4DYABZYLWOATPXUA3TA5CNFSM4HJFYWRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWLU5JQ#issuecomment-496455334, or mute the thread https://github.com/notifications/unsubscribe-auth/ABSYVTMCJUIPOBI7VRKTEWTPXUA3TANCNFSM4HJFYWRA .

benjdavenport commented 5 years ago

well, this might have introduced a crash... Spinning up a test box now with some logging, I'll post here :)

lminiero commented 5 years ago

If you can gather a libasan dump, that will help figure out if the cause is this, and if we have to move the reference decrease somewhere else. It shouldn't cause anything that an RTP forwarder without RTCP doesn't.

benjdavenport commented 5 years ago

sounds good, I'll definitely do that. Reverting to a build before the change and no crashes yet. I'll be sure to update here once I have a dump.

On Tue, May 28, 2019 at 5:01 PM Lorenzo Miniero notifications@github.com wrote:

If you can gather a libasan dump, that will help figure out if the cause is this, and if we have to move the reference decrease somewhere else. It shouldn't cause anything that an RTP forwarder without RTCP doesn't.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/meetecho/janus-gateway/issues/1605?email_source=notifications&email_token=ABSYVTPBABUGOLLV3QNX5UDPXWMT7A5CNFSM4HJFYWRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWNN2HQ#issuecomment-496688414, or mute the thread https://github.com/notifications/unsubscribe-auth/ABSYVTNVLZP2B5NH2ROGRL3PXWMT7ANCNFSM4HJFYWRA .