Execute janus_check_sessions if at least one of (session_timeout,reclaim_session_timeout) is set

[nicolasduteil]

This PR ensures that janus_check_sessions function will be called when one of the below conditions is true

• session_timeout > 0
• reclaim_session_timeout > 0

Current behaviour

In current master, janus_check_sessions function won't be executed unless session_timeout is > 0, regardless of the value of reclaim_session_timeout

• session_timeout = 0, reclaim_session_timeout = 0 : session is automatically destroyed upon ws disconnection

[Wed May  6 10:38:32 2020] [WSS-0x7f4534000fc0] WebSocket connection accepted
[Wed May  6 10:38:32 2020] [WSS-0x7f4534000fc0]   -- Ready to be used!
[Wed May  6 10:38:32 2020] Got a Janus API request from janus.transport.websockets (0x7f45340036f0)
[Wed May  6 10:38:32 2020] Creating new session: 2163588481368888; 0x7f4554002da0
[Wed May  6 10:38:43 2020] [WSS-0x7f4534000fc0] WS connection down, closing
[Wed May  6 10:38:43 2020] [WSS-0x7f4534000fc0] Destroying WebSocket client
[Wed May  6 10:38:43 2020] A janus.transport.websockets transport instance has gone away (0x7f45340036f0)
[Wed May  6 10:38:43 2020]   -- Session 2163588481368888 will be over if not reclaimed
[Wed May  6 10:38:43 2020]   -- Marking Session 2163588481368888 as over
[Wed May  6 10:38:43 2020] Destroying session 2163588481368888; 0x7f4554002da0

• session_timeout = 45, reclaim_session_timeout = 15 : session is automatically destroyed 15s after ws disconnection

[Wed May  6 11:01:27 2020] [WSS-0x7ff210000fc0] WebSocket connection accepted
[Wed May  6 11:01:27 2020] [WSS-0x7ff210000fc0]   -- Ready to be used!
[Wed May  6 11:01:27 2020] Got a Janus API request from janus.transport.websockets (0x7ff2100036f0)
[Wed May  6 11:01:27 2020] Creating new session: 5652647219916830; 0x7ff22c003b10
[Wed May  6 11:01:29 2020] [WSS-0x7ff210000fc0] WS connection down, closing
[Wed May  6 11:01:29 2020] [WSS-0x7ff210000fc0] Destroying WebSocket client
[Wed May  6 11:01:29 2020] A janus.transport.websockets transport instance has gone away (0x7ff2100036f0)
[Wed May  6 11:01:29 2020]   -- Session 5652647219916830 will be over if not reclaimed
[Wed May  6 11:01:29 2020]   -- Marking Session 5652647219916830 as over
[Wed May  6 11:01:29 2020] [WSS-0x7ff210000fc0]   -- closed
[Wed May  6 11:01:46 2020] Timeout expired for session 5652647219916830...
[Wed May  6 11:01:46 2020] Destroying session 5652647219916830; 0x7ff22c003b10

• session_timeout = 0, reclaim_session_timeout = 15 : session is never destroyed (although logs indicate that it will, if not reclaimed)

[Wed May  6 11:04:03 2020] [WSS-0x7fcc200015b0] WebSocket connection accepted
[Wed May  6 11:04:03 2020] [WSS-0x7fcc200015b0]   -- Ready to be used!
[Wed May  6 11:04:03 2020] Got a Janus API request from janus.transport.websockets (0x7fcc20003ce0)
[Wed May  6 11:04:03 2020] Creating new session 7123637785399315; 0x7fcc34001d70
[Wed May  6 11:04:04 2020] [WSS-0x7fcc200015b0] WS connection down, closing
[Wed May  6 11:04:04 2020] [WSS-0x7fcc200015b0] Destroying WebSocket client
[Wed May  6 11:04:04 2020] A janus.transport.websockets transport instance has gone away (0x7fcc20003ce0)
[Wed May  6 11:04:04 2020]   -- Session 7123637785399315 will be over if not reclaimed
[Wed May  6 11:04:04 2020]   -- Marking Session 7123637785399315 as over
[Wed May  6 11:04:04 2020] [WSS-0x7fcc200015b0]   -- closed

After 8min, session still exists

2020-05-06 11:11:20.566686
POST /admin HTTP/1.1
Host: 192.168.7.89:7088
User-Agent: curl/7.64.0
Accept: */*
Content-Length: 85
Content-Type: application/x-www-form-urlencoded

{"janus":"list_sessions","admin_secret":"xxxxxxx","transaction":"1588756280"}


2020-05-06 11:11:20.567487
HTTP/1.1 200 OK
Connection: Keep-Alive
Transfer-Encoding: chunked
Access-Control-Max-Age: 86400
Access-Control-Allow-Origin: *
Content-Type: application/json
Date: Wed, 06 May 2020 09:11:20 GMT

{
   "janus": "success",
   "transaction": "1588756280",
   "sessions": [
      7123637785399315
   ]
}

With the patch

• session_timeout = 0, reclaim_session_timeout = 15 : session is automatically destroyed 15s after ws disconnection

[Wed May  6 12:01:00 2020] [WSS-0x7f7f3c0015b0] WebSocket connection accepted
[Wed May  6 12:01:00 2020] [WSS-0x7f7f3c0015b0]   -- Ready to be used!
[Wed May  6 12:01:00 2020] Creating new session: 5273246303027227; 0x7f7f5c0025a0
[Wed May  6 12:01:01 2020] [WSS-0x7f7f3c0015b0] WS connection down, closing
[Wed May  6 12:01:01 2020] [WSS-0x7f7f3c0015b0] Destroying WebSocket client
[Wed May  6 12:01:01 2020] A janus.transport.websockets transport instance has gone away (0x7f7f3c003ce0)
[Wed May  6 12:01:01 2020]   -- Session 5273246303027227 will be over if not reclaimed
[Wed May  6 12:01:01 2020]   -- Marking Session 5273246303027227 as over
[Wed May  6 12:01:01 2020] [WSS-0x7f7f3c0015b0]   -- closed
[Wed May  6 12:01:18 2020] Timeout expired for session 5273246303027227...
[Wed May  6 12:01:18 2020] Destroying session 5273246303027227; 0x7f7f5c0025a0

[januscla]

Thanks for your contribution, @nicolasduteil! Please make sure you sign our CLA, as it's a required step before we can merge this.

[lminiero]

Thanks for the explanation, but not sure how that gets the session destroyed when the value is 0, if all you're doing is ensuring that janus_check_session is invoked when it's NOT 0?

[nicolasduteil]

Right now, in janus_check_sessions does the following :

1. destroy session if last activity occured more than session_timeout seconds ago
2. otherwise check if transport is gone AND if last activity occured more than reclaim_session_timeout seconds ago

janus-gateway/janus.c

Lines 615 to 617 in 52efcc4

	if ((now - session->last_activity >= (gint64)session_timeout * G_USEC_PER_SEC &&
	!g_atomic_int_compare_and_exchange(&session->timeout, 0, 1)) ||
	((g_atomic_int_get(&session->transport_gone) && now - session->last_activity >= (gint64)reclaim_session_timeout * G_USEC_PER_SEC) &&

:

But if session_timeout == 0, function will never perform above checks which means that a websocket session with goen transport will never be destroyed

[lminiero]

Ah, right, I wasn't looking at the whole check, which I remembered was an AND and is an OR instead... and to think I wrote the code 🤭 I'll give a review and a check later 👍

[lminiero]

Looks good, merging 👍