Azure / azure-remote-rendering

SDK and samples for Azure Remote Rendering
MIT License
106 stars 38 forks source link

Null ref inside OnConnectionStatusChanged if session disconnects and running on device #34

Closed WikkidEdd closed 3 years ago

WikkidEdd commented 3 years ago

After upgrading to SDK version 1.0.6 we've been getting a null ref exception inside OnConnectionStatusChanged. The logs don't have line numbers, but we can see that the user failed to connect with "TransportConnectionFailed" followed by a null reference exception inside OnConnectionStatusChanged.

Looking at the code we think the issue is related to changes allowing skybox support: if (status == ConnectionStatus.Disconnected) { // Making sure main camera clearing is used again (Disabled in updateLocal // when we actually receive remote frames). _camera.clearFlags = _proxyCamera.clearFlags; _camera.backgroundColor = _proxyCamera.backgroundColor; _entityFlushPending = true; OnSessionUpdate?.Invoke(SessionUpdate.SessionDisconnected); }

It looks like when running on device _proxyCamera never gets assigned a value, so when running through the above code it would throw a null ref when trying to access the camera properties.

I would also expect it to throw this exception if disconnecting manually too, but we clear the session straight after manual disconnect so Update won't be called on it to pump that callback.

jumeder commented 3 years ago

Hi @WikkidEdd, thanks for your bug report and apologies for the delayed reply. Do you get this null reference exception on Desktop or on HoloLens? Looks like there is a check missing in the code part you pasted above. We'll fix this in the repo. As a workaround in the meantime, I think it's fine if you just guard the two assignments against a null _proxyCamera locally.

WikkidEdd commented 3 years ago

Sorry, I wasn't explicit. When I say device I meant Hololens and it's only on Hololens as _proxyCamera get populated correctly when running on Desktop.

I've already added that workaround, just wanted to make you aware so you can fix it in the repo đź‘Ť

jumeder commented 3 years ago

No worries, thanks for confirming.

jumeder commented 3 years ago

We fixed the issue in the latest release 1.0.10.