Double execution of event listeners after remounting OTSession

[evillemez]

Bug Report

Current behavior

If OTSession is remounted, event listeners it receives from the parent component will be called twice for a time. I believe this is related to #262, but am not 100% sure.

Steps to reproduce

• Create a component that wraps OTSession and defines event listeners for connectionCreated, streamCreated, connectionDestroyed and streamDestroyed.
• This parent component should be configured to mount OTSession only in the case where the NetInfo.isConnected status is true.
• Join a call
• On one side (we'll call this the local user), force a network error by disabling wifi, which will result in unmounting OTSession
• On the local side the disconnect will be seen immediately
• On the remote side, the disconnect will not be seen until 1 min has passed (this is the time limit Opentok is enforcing before considering a connection dead and cleaning it up)
• Before the minute has elapsed (say after 20s), re-enable the wifi on the local side
• OTSession will remount and the two sides will be reconnected
• At the 1min mark after wifi was originally disabled, on the local side, you will see the connectionDestroyed, streamDestroyed events fire from the original connection. On the remote side you will see a streamDestroyed and connectionDestroyed event from the old connection as well.

What is the current bug behavior?

Registered event handlers are firing from an instance of OTSession which was previously unmounted.

What is the expected correct behavior?

Event handlers from old instances should be fully cleared, only triggered for the most recently mounted instance.

Other thoughts

I think part of the issue here may be shared state at the native level. If one wanted to have multiple sessions going (see #218) it is currently not possible, but another side effect of the shared state is that even in the case of only having one instance at a time in the RN layer, if a user has had to disconnect and reconnect, it is resulting in conflicting from the previously destroyed instance.

The effect of this at the app level is that it's making it impossible to tell to the difference between real disconnect events, and phantom disconnect events from a previous instance. So we have cases where two users get reconnected, but our app thinks they are disconnected because of the previous connection/streams timing out.

I've added extra checks to verify the specific stream and connection IDs, but it's not enough, because on the local side it will still see a streamDestroyed and connectionDestroyed event for the stream/connection that is actually still connected in the new instance, because the remote side still has the same stream/connection as it did before the disconnect on the local side.

[msach22]

@evillemez Thanks for filing this. The way I'm reading this, I'm thinking that the events are triggering in the second instance because the first local user has officially disconnected. To verify this, could you please mount the second instance of OTSession without any event handlers and see if the first ones are fired?

[evillemez]

That's what I think is happening too. I'll try that test as soon as I can, it may be a little tricky to set up.

[msach22]

Thanks @evillemez - please keep me updated

[niraj1997]

I am also facing the same issue.

Is there any update on it?

[ASerga]

@msach22, is there any update on it?

[evillemez]

@msach22 I'm still sure this is an issue, not finding any other explanation for the extra disconnect signals.

Is there more information anyone of us, me, or @ASerga or @niraj1997 can provide here? It's a serious issue for us unfortunately, because we unmount/remount the session in the case of a party losing their network connection, or if the session/publisher/subscriber trigger an error.

[evillemez]

@msach22 @ggoldens I saw the PR referencing this - is this potentially fixed in 0.12.0?

[ggoldens]

Hi @evillemez sorry for the late reply. Yes, it should be fixed in 0.12.0. Could you please confirm if after upgrading it works as expected?
Thanks

[ggoldens]

This should be fixed in opentok-react-native >= 0.12.0. Closing this issue.

[julianoddreis]

@ggoldens Not fixed yet. I'm using 0.12.1 and still not clearing the event handlers.

[TomasUhrik]

I can confirm that 0.12.1 still has this issue.

To elaborate a bit: I think that all unmounted sessions still "live" somewhere as @evillemez nicely says it here:

Registered event handlers are firing from an instance of OTSession which was previously unmounted.

It's not limited to only one session though. Meaning that if you unmount 5 sessions and mount a new one, then any upcoming events get called 6 times.

[msach22]

@TomasUhrik Are you only seeing this on iOS or on Android as well?

[julianoddreis]

This fork resolves this issue: https://github.com/naveteam/opentok-react-native

[msach22]

@julianoddreis - looks like the diff is : nativeEvents.removeAllListeners(eventType);

I had a different theory on how there might be a memory leak in the native iOS SDK which is why I asked if it was for iOS or Android.

[evillemez]

Thanks @TomasUhrik and @julianoddreis for the extra info.

@msach22 and @ggoldens - can we re-open the issue until we’re more sure it’s fixed?

[TomasUhrik]

@TomasUhrik Are you only seeing this on iOS or on Android as well?

@msach22 happens on Android as well.

Thanks for picking up on this so fast 👍

[evillemez]

@julianoddreis Any chance you could PR your fix to this repo?

[ggoldens]

Reopening this issue. This is going to be revisited.

[julianoddreis]

#374

[adaerodriguez]

Please, is there any change on this issue?

[SaveYourTime]

Same here, still running into this on "opentok-react-native": "^0.13.0"

[codeMeSid]

@evillemez Thanks for filing this. The way I'm reading this, I'm thinking that the events are triggering in the second instance because the first local user has officially disconnected. To verify this, could you please mount the second instance of OTSession without any event handlers and see if the first ones are fired?

Found this to be a very elegant solution , but to further make sure old session events do not fire, i think it will be better to just disconnect or destroy past session manually and then immediately initialise a new session

[evillemez]

@codeMeSid Can you describe a little more? I'm not quite following what the solution is. In our case we have unmounted the session, then mounted it again. That's supposed to trigger a disconnect/destroy of the original session, and initialize a new one - but that's not the behavior we're actually getting.

When you say "disconnect or destroy the past session manually" - how exactly are you doing that?

[codeMeSid]

const createSession = () => {
      if (session) {
        session.off('streamCreated', onStreamCreated);
        session.off('streamDestroyed', onStreamDestroyed);
        session.disconnect();
      }
      const newSession = initSession(apiKey, sessionId);
      newSession?.on('streamCreated', onStreamCreated);
      newSession?.on('streamDestroyed', onStreamDestroyed);
      newSession?.connect(token, error => error && handleError(error));
      setSession(newSession);
}

so now even all your existing publisher and subscribers are re-initialised

[evillemez]

@codeMeSid I may have missed something... Where is session.on() and session.off() coming from? That looks like usage from the opentok.js library. Our event listeners are wired up by passing event handlers as a property to the Opentok React Native components.

Is there an alternate way for us to register/unregister our event listeners instead of passing props to the OTSession, OTPublisher, and OTSubscriber?

@ggoldens @msach22 Is this a thing?

That said - the opentok.js lib does allow unregistering all event listeners by just calling session.off(), and not specifying any specific event. If this library exposed an equivalent API, that would at least let us solve the problem in our code because we could potentially call it before unmounting our root session component.

EDIT: Adding a related question. From the current docs, there is an example of getting a ref to the session component:

      this.otSessionRef.current.signal({
        data: '',
        to: '', // optional - connectionId of connected client you want to send the signal to
        type: '', // optional
      })

Is it documented anywhere what the API being exposed there is? Are there any other methods we could call? Also - is this the recommended way of signaling now instead of using the signal prop?

[nicoladj77]

I can still see this on 0.14.0. this is my code:

				{showOTSession && (
					<OTSession
						apiKey={params.apikey}
						sessionId={params.sessionId}
						token={params.token}
						eventHandlers={eventHandlers}
					>
						<OTPublisher style={{ width: 200, height: 200 }} />
					</OTSession>
				)}

showOTSession is a functional component state and can be triggered on and off. I need to do this as I have to reload the session wit different parameters.

[Alan-Billi]

Hello,

we're still facing this issue with the last release.
It seems that the OTSubscriber view is not unmounted which result in multiple DOM elements created when a user is disconnecting/reconnecting to a session.

Thanks.

[enricop89]

Hi all, we have deployed 0.17.1 with the changes from #374.

Please let us know if it fixes the issue. Thanks again to everyone 😀

[hardikjs]

@enricop89 It's still happening with me and i am using version 0.17.2

For me signal events are duplicated so it means event listener called 2 times and i have stored event listener object sessionEventHandlers in the constructor like below

this.sessionEventHandlers = {
   signal: (event) => {
       ...
   },
   ...
   // many more events here
}

I am remounting OTSession as @nicoladj77 showed above.

[evillemez]

Just curious of anyone is still experiencing this on the most recent versions?

[TalhaAhsan]

Closing this issue as it is outdated. You can open a new ticket if this issue still persists.

[vineus]

I might have found the issue:

when OTSession unmounts:

opentok-react-native/src/OTSession.js

Lines 57 to 59 in 28ff333

	componentWillUnmount() {
	this.disconnectSession();
	}

a disconnectSession() function is called:

opentok-react-native/src/OTSession.js

Lines 84 to 90 in 28ff333

	OT.disconnectSession(this.props.sessionId, (disconnectError) => {
	if (disconnectError) {
	this.otrnEventHandler(disconnectError);
	} else {
	const events = sanitizeSessionEvents(this.props.sessionId, this.props.eventHandlers);
	removeNativeEvents(events);
	}

when there is an error during the disconnect (eg: "the network is off"), the events are never cleaned up.

Calling the events cleanup during my component cleanup manually solved the issue for me:

import { removeNativeEvents } from "opentok-react-native/src/OT";
import { sanitizeSessionEvents } from "opentok-react-native/src/helpers/OTSessionHelper";

// ...

  useEffect(() => {
    return () => {
      const events = sanitizeSessionEvents(sessionId, sessionEventHandlers);
      removeNativeEvents(events);

    };
  }, []);

but it uses non-exported functions.

@TalhaAhsan : why the else in the disconnect function? don't we always want to remove the events listener on component unmount?

(got the tip from this comment: opentok/accelerator-core-js#97 (comment) )

[enricop89]

@vineus thanks for the debugging, would you be able to test #625?

[vineus]

@enricop89 : I tried the fix, unfortunately this doesn't solve the issue I have (reconnecting after a full disconnection) because the OTSession component doesn't seem to unmount at all when there was a disconnection event.

So there might be something else at play here preventing the component to unmount (another condition somewhere ?)

I won't dig more into this as the (ugly) workaround proposed works for us in most cases (we can control the unmounting of the component embedding OTSession)

edit: random thought: I suppose that when the network is "off", this line here:

opentok-react-native/src/OTSession.js

Line 84 in 28ff333

OT.disconnectSession(this.props.sessionId, (disconnectError) => {

takes "some times" to execute synchronously (waiting for a timeout on disconnect in the native code) doesn't it ?

That would explain the behaviour I see.

Possible solutions:

• disconnecting the events before the session
• 0ms timeout on session disconnection when there is no network

just hypothesis here, I don't know the code at all 🙈

[enricop89]

@vineus ok so can you share the steps to reproduce, please? Is it just connect to a session, disconnect the network and trigger a reconnect event?

[vineus]

@vineus ok so can you share the steps to reproduce, please? Is it just connect to a session, disconnect the network and trigger a reconnect event?

1. connect a session
2. disconnect the network
3. wait for the session disconnect event (takes more than 30 second - this is not the "sessiontReconnecting" event this is the "sessionDisconnected" one)
4. unmount the OTSession component (eg: by displaying a loader "you lost your internet connection, attempting to reconnect..." or by navigating out of the current screen)
5. reconnect the network
6. remount the OTSession component (stop displaying the loader or navigate back to the session screen)

when the component re-mounts, the events are not attached to the current OTSession component. The session works, the stream is published (and seen by the remote party) but events triggering have no effect.

[evillemez]

There's still an open PR referencing this issue which hasn't been merged. I feel like this issue really should be re-opened, as it seems to still be affecting multiple teams.