WindowsMixedRealityDeviceManager can oversubscribe to Unity's InteractionManager events

[nuernber]

Describe the bug

WindowsMixedRealityDeviceManager.Enable() can be called multiple times, thus leading to many subscriptions to Unity's InteractionManager events. This can manifest when calling CoreServices.InputSystem.Enable() twice. A NullReferenceException can then occur if CoreServices.InputSystem.Disable() is called and a hand input source is then detected.

The over subscription happens here: https://github.com/microsoft/MixedRealityToolkit-Unity/blob/06a06778e38da622b37cc299a93f16e143b7bdeb/Assets/MRTK/Providers/WindowsMixedReality/XR2018/WindowsMixedRealityDeviceManager.cs#L378-L381

To reproduce

Steps to reproduce the behavior:

1. Call CoreServices.InputSystem.Enable() twice.
2. Call CoreServices.InputSystem.Disable(). You may need to call this while a hand is detected, not sure.
3. Bring your hand outside HL's tracking field of view and then bring it back in.
4. Notice a NullReferenceException:

System.NullReferenceException: Object reference not set to an instance of an object.
BaseWindowsMixedRealitySource Microsoft.MixedReality.Toolkit.WindowsMixedReality.Input.WindowsMixedRealityDeviceManager:GetOrAddController (InteractionSource, Boolean)+0x253 at C:\github\protospace\unity\protospace\Assets\MRTK\Providers\WindowsMixedReality\XR2018\WindowsMixedRealityDeviceManager.cs:[718:17-718:92] C#
Void Microsoft.MixedReality.Toolkit.WindowsMixedReality.Input.WindowsMixedRealityDeviceManager:GetOrAddController (InteractionSourceState)+0x38 at C:\github\protospace\unity\protospace\Assets\MRTK\Providers\WindowsMixedReality\XR2018\WindowsMixedRealityDeviceManager.cs:[409:17-409:84]   C#
Void Microsoft.MixedReality.Toolkit.WindowsMixedReality.Input.WindowsMixedRealityDeviceManager:InteractionManager_InteractionSourceDetected (InteractionSourceDetectedEventArgs)+0x8 at C:\github\protospace\unity\protospace\Assets\MRTK\Providers\WindowsMixedReality\XR2018\WindowsMixedRealityDeviceManager.cs:[759:111-759:141]    C#
Void UnityEngine.XR.WSA.Input.InteractionManager:OnSourceEvent (EventType, IntPtr, InteractionSourcePressType)+0x53 at C:\buildslave\unity\build\Runtime\VR\HoloLens\ScriptBindings\InteractionManager.bindings.cs:440  C#

The line where this is thrown is in this for-loop, where detectedController.InputSource.Pointers[0] is null:

for (int i = 0; i < detectedController.InputSource?.Pointers?.Length; i++)
{
   detectedController.InputSource.Pointers[i].Controller = detectedController;
}

Expected behavior

• If WindowsMixedRealityDeviceManager.Enable() gets called somehow twice, it shouldn't subscribe a second time to any InteractionManager events.
• If CoreServices.InputSystem.Disable() is called, we shouldn't be receiving input events anymore.

Your setup (please complete the following information)

• Unity Version 2018.4.23f1
• MRTK Version v2.4

Target platform (please complete the following information)

• HoloLens 2

Additional context

There might be similar issues with other parts of the code after CoreServices.InputSystem.Enable(); gets called more than once, but this is the one that I've discovered.

[keveleigh]

Do you have an example of when Enable might be called twice? It should only really be called by the MRTK lifecycle manager, which shouldn't enable it multiple times in a row.

We can try to block against it being called too many times in a row, but I think mostly it's just that manually calling Enable and Disable isn't a fully supported scenario.

[nuernber]

Yes, this is definitely a rare, corner case that you might not want to support. It happened in the context of an app that had its own interaction system and I wanted to enable/disable MRTK's interaction system at certain points. I discovered it by accidentally calling Enable too many times. It's very rare, so it's understandable if you don't want to support this situation. I suppose most apps will only use MRTK's interaction system and not something else simultaneously.

[stale[bot]]

This issue has been marked as stale by an automated process because it has not had any recent activity. It will be automatically closed in 30 days if no further activity occurs. If this is still an issue please add a new comment with more recent details and repro steps.

[IssueSyncBot]

We appreciate your feedback and thank you for reporting this issue.

Microsoft Mixed Reality Toolkit version 2 (MRTK2) is currently in limited support. This means that Microsoft is only fixing high priority security issues. Unfortunately, this issue does not meet the necessary priority and will be closed. If you strongly feel that this issue deserves more attention, please open a new issue and explain why it is important.

Microsoft recommends that all new HoloLens 2 Unity applications use MRTK3 instead of MRTK2.

Please note that MRTK3 was released in August 2023. It features an all new architecture for developing rich mixed reality experiences and has a minimum requirement of Unity 2021.3 LTS. For more information about MRTK3, please visithttps://www.mixedrealitytoolkit.org.

Thank you for your continued support of the Mixed Reality Toolkit!