Crash when exiting VR on Master Branch #0fe1f286

[wyzwon]

Description:
Upon exiting VR, I get the following error in the browser console:
Uncaught TypeError: Cannot read properties of null (reading 'removeEventListener') at XRSession.onSessionEnd (three.js:18792:1)
This error leaves the page entirely unresponsive and does not show up in release 1.3.0

• A-Frame Version: Master build #0fe1f286
• Platform / Device: gen 1 HTC Vive / Windows 10 / Steam VR / Chrome 106.0.5249.119
• Reproducible Code Snippet or URL:
  Enter VR mode then hit Esc to exit VR. The scene should then become un-responsive.
  Any A-frame scene using build #0fe1f286. Such as from:

<script src="https://cdn.jsdelivr.net/gh/aframevr/aframe@c27d4a2/dist/aframe-master.min.js"></script>

[dmarcos]

Chrome WebXR support on desktop has had a lot quirks since transition to OpenXR. It would be worth trying on Quest / Oculus browser and see it the issue reproduces consistently.

[wyzwon]

I was able to exit VR on the Quest 2 without any fatal crash on both the standalone Quest browser and via a desktop link. I also reconfirmed the issue still persists for the Vive in Chrome version 107.0.5304.88

[dmarcos]

Chrome WebXR on desktop has a lot of quirks. Might want to read #4709 and add your experience if not covered. Not sure if there's anything to be done on A-Frame side. Don't have a Windows machine to test out. If you can reproduce consistently a little debugging will be appreciated. It looks like the error happens at the THREE level. Can set a breakpoint before the error occurs and inspect the variables.

[wyzwon]

I have narrowed down the introduction of the bug to between c937479 and d8de409. The only code changes between these two builds appear to be:

1. An update to wasd-controls (480a303)
2. A super-three bump from 0.137.0 to 0.141.0 (837023c)

I am also confirming that the error still persists in version 1.4.0 running Chrome 108.0.5359.125 (Official Build) (64-bit)/Steam VR beta 1.25.2

I did notice that even in older versions, the relaunched game is a little twitchy, but I suspect it's unrelated to the bug at hand.

[vincentfretin]

I remember I saw this error on Quest 1 Meta browser sometimes while I was developing. I didn't really care at that time because I just refreshed the page between each VR test. So I don't know if the page was also unresponsive or not in my case.
If you do a build with this change @wyzwon Hubs-Foundation@0fa1418 does it fix the issue?

[vincentfretin]

My guess is there may be a race condition between the threejs cleanup and aframe cleanup. That's why I don't always see this error.
When I looked at WebXRManager commits history https://github.com/mrdoob/three.js/commits/dev/src/renderers/webxr/WebXRManager.js I didn't see a recent change in r141 related to cleanup as @netpro2k mentioned in the comment.
Also does this vrManager.setSession(null) really did something at all? We didn't await it, or call .then() on it, setSession is an async function.

[vincentfretin]

aframe/src/core/scene/a-scene.js

Lines 405 to 407 in fbfb4de

	this.xrSession.end().then(function () {}, function () {});
	this.xrSession = undefined;
	vrManager.setSession(null);

session is set to null after the end event here:
https://github.com/mrdoob/three.js/blob/4fa01465053d3d95e5c6e05fee701b1b217878aa/src/renderers/webxr/WebXRManager.js#L262
https://github.com/mrdoob/three.js/blob/4fa01465053d3d95e5c6e05fee701b1b217878aa/src/renderers/webxr/WebXRManager.js#L175

setSession(null) is setting session = null here:
https://github.com/mrdoob/three.js/blob/4fa01465053d3d95e5c6e05fee701b1b217878aa/src/renderers/webxr/WebXRManager.js#L248-L250

It seems the issue will trigger if setSession(null) is called before onSessionEnd is called via the end event.

But if onSessionEnd is already setting session = null, we don't even need setSession(null) that is just doing session = null too as far as I can tell.

[vincentfretin]

I found it, in three r132 session = null was added
mrdoob/three.js@daffe80#diff-a27b9d3ba68457069cc8ff521208ae9d0cc6d9256f6f48ada6316fc2d732ba7e
so the setSession(null) is not needed anymore.
I created #5198 to fix the issue.

aframe 1.3.0 was using r137 so the issue is not new.

[dmarcos]

Thanks @wyzwon for the extra info. it was really helpful

[wyzwon]

aframe 1.3.0 was using r137 so the issue is not new.

Odd, I consistently only get the error report and complete freeze post 1.3.0, though I guess if the THREE update weighted the race condition enough that could work out. I will certainly test it post fix though. Thanks for all your work!

[wyzwon]

The error is not showing up in 1.4.1 👍

I am still getting a warning (twice each time the XR session ends), but it's clearly a different issue, so I will try to get a new issue report up for it.

Successive relaunches are also still more and more jittery until the scene stops redrawing entirely, but this appears to be true for ImmersiveWeb demos as well so I suspect it's at least outside the scope of AFrame.