Fix various process lifecycle trigger bugs in the Actions addon
I installed the dev build so I could automatically switch to my headphones when Minecraft launches and switch back to my speakers once it closes. I was getting odd crashes and decided to investigate, and ended up finding three things to fix.
I was originally going to split this into multiple PRs but noticed all changes were in the same file. If you'd like multiple PRs (or even just multiple commits in the same PR), let me know and I can recreate things separately.
List of fixes, in the order I found and fixed them:
-
ProcessWatcher.FoundNewRelevantProcess- Pass the process ID into the thread instead of theProcessobject- Fixes a crash when accessing most things on the
Processobject inside this thread - I'm not sure why this happens since it's just a parameter, but most of the
Processvariables seem reset when called inside theThread(seen via debugger)- Technically the internal process ID variable is valid, but accessing it without reflection doesn't work either since the public property relies on other values which were reset before returning
- The fix uses the passed in process ID to get the
Processobject again (viaProcess.GetProcessById), which works fine.
- Fixes a crash when accessing most things on the
-
ProcessWatcher.RegisterStart- FetchWatcherInfothe same way thatRegisterStopdoes- Fixes actions triggered via "process start" not ever actually running
-
RegisterStopwould add the newWatcherInfoto_info, whileRegisterStartonly created a new one and did not add it. - Since it wasn't in
_info,FoundNewRelevantProcesswas unable to find the information necessary.
-
ProcessWatcher.FoundNewRelevantPRoces- Defer invoking the stop callbacks to the UI thread, instead of directly in the "wait for process to stop" background thread.- Fixes a crash from changing the default playback device on a "process ends" trigger.
- Without this fix I was getting the following exception:
-
System.NotSupportedException - Message: "This type of CollectionView does not support changes to its SourceCollection from a thread different from the Dispatcher thread."
- Stack trace below (from before I made this fix, but did make the previous two)
-
[External Code]
> EarTrumpet.dll!EarTrumpet.UI.ViewModels.FlyoutViewModel.OnDefaultPlaybackDeviceChanged(object sender, EarTrumpet.UI.ViewModels.DeviceViewModel e) Line 168 C#
EarTrumpet.dll!EarTrumpet.UI.ViewModels.DeviceCollectionViewModel.SetDefault(EarTrumpet.UI.ViewModels.DeviceViewModel device) Line 75 C#
EarTrumpet.dll!EarTrumpet.UI.ViewModels.DeviceCollectionViewModel.OnDefaultChanged(object sender, EarTrumpet.DataModel.Audio.IAudioDevice newDevice) Line 63 C#
EarTrumpet.dll!EarTrumpet.DataModel.WindowsAudio.Internal.AudioDeviceManager.Default.set(EarTrumpet.DataModel.Audio.IAudioDevice value) Line 34 C#
EarTrumpet.dll!EarTrumpet.Actions.DataModel.Processing.ActionProcessor.Invoke(EarTrumpet.Actions.DataModel.Serialization.BaseAction a) Line 30 C#
EarTrumpet.dll!EarTrumpet.Actions.EarTrumpetActionsAddon.TriggerAction.AnonymousMethod__23_0(EarTrumpet.Actions.DataModel.Serialization.BaseAction a) Line 125 C#
[External Code]
EarTrumpet.dll!EarTrumpet.Actions.EarTrumpetActionsAddon.TriggerAction(EarTrumpet.Actions.DataModel.Serialization.EarTrumpetAction action) Line 125 C#
EarTrumpet.dll!EarTrumpet.Actions.EarTrumpetActionsAddon.OnTriggered(EarTrumpet.Actions.DataModel.Serialization.BaseTrigger trigger) Line 119 C#
EarTrumpet.dll!EarTrumpet.Actions.DataModel.Processing.TriggerManager.Register.AnonymousMethod__1() Line 55 C#
EarTrumpet.dll!EarTrumpet.Actions.DataModel.ProcessWatcher.FoundNewRelevantProcess.AnonymousMethod__10_2(System.Action s) Line 92 C#
[External Code]
EarTrumpet.dll!EarTrumpet.Actions.DataModel.ProcessWatcher.FoundNewRelevantProcess.AnonymousMethod__0() Line 92 C#
[External Code]
I kinda would like to be able to use a specific file path in addition to just the process name for this trigger (since at the moment any java application will change to my headphones, not just ones using the java instance I use for Minecraft), but I imagine that should definitely be a separate PR (if it's even an acceptable feature to merge).
Wow sounds awesome, will get this reviewed ASAP. Am a bit behind due to back to back events and vacation.
Thanks again! (Will work the CI issues separately.)