EarTrumpet icon indicating copy to clipboard operation
EarTrumpet copied to clipboard

Fix various process lifecycle trigger bugs in the Actions addon

Open spacechase0 opened this issue 8 months ago • 2 comments

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 the Process object
    • Fixes a crash when accessing most things on the Process object inside this thread
    • I'm not sure why this happens since it's just a parameter, but most of the Process variables seem reset when called inside the Thread (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 Process object again (via Process.GetProcessById), which works fine.
  • ProcessWatcher.RegisterStart - Fetch WatcherInfo the same way that RegisterStop does
    • Fixes actions triggered via "process start" not ever actually running
    • RegisterStop would add the new WatcherInfo to _info, while RegisterStart only created a new one and did not add it.
    • Since it wasn't in _info, FoundNewRelevantProcess was 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).

spacechase0 avatar May 29 '25 03:05 spacechase0

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar May 29 '25 03:05 CLAassistant

Wow sounds awesome, will get this reviewed ASAP. Am a bit behind due to back to back events and vacation.

riverar avatar May 29 '25 08:05 riverar

Thanks again! (Will work the CI issues separately.)

riverar avatar Jun 19 '25 17:06 riverar