Skip to content

Commit

Permalink
fix: multi threading issue when running many keybinding commands at once
Browse files Browse the repository at this point in the history
  • Loading branch information
lars-berger committed Sep 5, 2023
1 parent 12614cf commit 024f68a
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 115 deletions.
2 changes: 1 addition & 1 deletion GlazeWM.Domain/DependencyInjection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ public static IServiceCollection AddDomainServices(this IServiceCollection servi
services.AddSingleton<ICommandHandler<EvaluateUserConfigCommand>, EvaluateUserConfigHandler>();
services.AddSingleton<ICommandHandler<RegisterKeybindingsCommand>, RegisterKeybindingsHandler>();
services.AddSingleton<ICommandHandler<ReloadUserConfigCommand>, ReloadUserConfigHandler>();
services.AddSingleton<ICommandHandler<RunWithSubjectContainerCommand>, RunWithSubjectContainerHandler>();
services.AddSingleton<ICommandHandler<CloseWindowCommand>, CloseWindowHandler>();
services.AddSingleton<ICommandHandler<IgnoreWindowCommand>, IgnoreWindowHandler>();
services.AddSingleton<ICommandHandler<ManageWindowCommand>, ManageWindowHandler>();
services.AddSingleton<ICommandHandler<MoveWindowCommand>, MoveWindowHandler>();
services.AddSingleton<ICommandHandler<ResizeWindowCommand>, ResizeWindowHandler>();
services.AddSingleton<ICommandHandler<RunWindowRulesCommand>, RunWindowRulesHandler>();
services.AddSingleton<ICommandHandler<ResizeWindowBordersCommand>, ResizeWindowBordersHandler>();
services.AddSingleton<ICommandHandler<SetFloatingCommand>, SetFloatingHandler>();
services.AddSingleton<ICommandHandler<SetMaximizedCommand>, SetMaximizedHandler>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ public CommandResponse Handle(RegisterKeybindingsCommand command)
foreach (var keybindingConfig in command.Keybindings)
{
// Format command strings defined in keybinding config.
var formattedCommandStrings = keybindingConfig.CommandList.Select(
commandString => CommandParsingService.FormatCommand(commandString)
var commandStrings = keybindingConfig.CommandList.Select(
CommandParsingService.FormatCommand
);

// Register all keybindings for a command sequence.
Expand All @@ -56,29 +56,7 @@ public CommandResponse Handle(RegisterKeybindingsCommand command)
if (_windowService.IgnoredHandles.Contains(GetForegroundWindow()))
return;

var subjectContainer = _containerService.FocusedContainer;
var subjectContainerId = subjectContainer.Id;

// Invoke commands in sequence on keybinding press.
foreach (var commandString in formattedCommandStrings)
{
// Avoid calling command if container gets detached. This is to prevent crashes
// for edge cases like ["close", "move to workspace X"].
if (subjectContainer.IsDetached())
return;

var parsedCommand = _commandParsingService.ParseCommand(
commandString,
subjectContainer
);

// Use `dynamic` to resolve the command type at runtime and allow multiple dispatch.
_bus.Invoke((dynamic)parsedCommand);

// Update subject container in case the reference changes (eg. when going from a tiling to a
// floating window).
subjectContainer = _containerService.GetContainerById(subjectContainerId);
}
_bus.Invoke(new RunWithSubjectContainerCommand(commandStrings));
}
catch (Exception e)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ public CommandResponse Handle(ReloadUserConfigCommand command)
foreach (var window in _windowService.GetWindows())
{
var windowRules = _userConfigService.GetMatchingWindowRules(window);
_bus.Invoke(new RunWindowRulesCommand(window, windowRules));
var windowRuleCommands = windowRules
.SelectMany(rule => rule.CommandList)
.Select(CommandParsingService.FormatCommand);

// Run matching window rules.
_bus.Invoke(new RunWithSubjectContainerCommand(windowRuleCommands, window));
}

// Redraw full container tree.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
using GlazeWM.Domain.Containers;
using GlazeWM.Domain.Containers.Commands;
using GlazeWM.Domain.UserConfigs.Commands;
using GlazeWM.Domain.UserConfigs.Events;
using GlazeWM.Domain.Windows;
using GlazeWM.Domain.Windows.Commands;
using GlazeWM.Domain.Workspaces.Commands;
using GlazeWM.Infrastructure.Bussing;

namespace GlazeWM.Domain.UserConfigs.CommandHandlers
{
internal sealed class RunWithSubjectContainerHandler
: ICommandHandler<RunWithSubjectContainerCommand>
{
private readonly Bus _bus;
private readonly CommandParsingService _commandParsingService;
private readonly ContainerService _containerService;

public RegisterKeybindingsHandler(
Bus bus,
CommandParsingService commandParsingService,
ContainerService containerService)
{
_bus = bus;
_commandParsingService = commandParsingService;
_containerService = containerService;
}

public CommandResponse Handle(RunWithSubjectContainerCommand command)
{
var commandStrings = command.CommandStrings;
var subjectContainer =
command.SubjectContainer ?? _containerService.FocusedContainer;

var subjectContainerId = subjectContainer.Id;

// Invoke commands in sequence.
foreach (var commandString in commandStrings)
{
// Avoid calling command if container gets detached. This is to prevent crashes
// for edge cases like ["close", "move to workspace X"].
if (subjectContainer?.IsDetached() != false)
return;

var parsedCommand = _commandParsingService.ParseCommand(
commandString,
subjectContainer
);

// Use `dynamic` to resolve the command type at runtime and allow multiple
// dispatch.
_bus.Invoke((dynamic)parsedCommand);

// Update subject container in case the reference changes (eg. when going from a
// tiling to a floating window).
subjectContainer = _containerService.GetContainerById(subjectContainerId);
}

return CommandResponse.Ok;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
using GlazeWM.Infrastructure.Bussing;

namespace GlazeWM.Domain.UserConfigs.Commands
{
public class RunWithSubjectContainerCommand : Command
{
public List<string> CommandStrings { get; }
public Container SubjectContainer { get; }

public RunWithSubjectContainerCommand(
List<string> commandStrings,
Container subjectContainer)
{
CommandStrings = commandStrings;
SubjectContainer = subjectContainer;
}

public RunWithSubjectContainerCommand(List<string> commandStrings)
{
CommandStrings = commandStrings;
}
}
}
30 changes: 17 additions & 13 deletions GlazeWM.Domain/Windows/CommandHandlers/ManageWindowHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,15 @@ public CommandResponse Handle(ManageWindowCommand command)
else
_bus.Invoke(new AttachContainerCommand(window, targetParent, targetIndex));

// The OS might spawn the window on a different monitor to the target parent, so adjustments
// might need to be made because of DPI.
var monitor = _monitorService.GetMonitorFromHandleLocation(windowHandle);
if (MonitorService.HasDpiDifference(monitor, window.Parent))
window.HasPendingDpiAdjustment = true;
var windowRules = _userConfigService.GetMatchingWindowRules(window);
var windowRuleCommands = windowRules
.SelectMany(rule => rule.CommandList)
.Select(CommandParsingService.FormatCommand);

// Set the newly added window as focus descendant. This means the window rules will be run as
// if the window is focused.
_bus.Invoke(new SetFocusedDescendantCommand(window));

// Run matching window rules.
var windowRules = _userConfigService.GetMatchingWindowRules(window);
_bus.Invoke(new RunWindowRulesCommand(window, windowRules));
_bus.Invoke(new RunWithSubjectContainerCommand(windowRuleCommands, window));

// Update window in case the reference changes.
window = _windowService.GetWindowByHandle(window.Handle);
Expand All @@ -82,12 +78,12 @@ public CommandResponse Handle(ManageWindowCommand command)
if (shouldRedraw)
_bus.Invoke(new RedrawContainersCommand());

// Set OS focus to the newly added window in case it's not already focused.
_bus.Invoke(new SetNativeFocusCommand(window));

_logger.LogWindowEvent("New window managed", window);
_bus.Emit(new WindowManagedEvent(window));

// Set OS focus to the newly added window in case it's not already focused.
_bus.Invoke(new SetNativeFocusCommand(window));

return CommandResponse.Ok;
}

Expand All @@ -106,7 +102,7 @@ private static Window CreateWindow(IntPtr windowHandle, Container targetParent)
var isResizable = WindowService.HandleHasWindowStyle(windowHandle, WindowStyles.ThickFrame);

// TODO: Handle initialization of maximized and fullscreen windows.
return windowType switch
var window = windowType switch
{
WindowType.Minimized => new MinimizedWindow(
windowHandle,
Expand All @@ -128,6 +124,14 @@ private static Window CreateWindow(IntPtr windowHandle, Container targetParent)
WindowType.Fullscreen => throw new ArgumentException(null, nameof(windowHandle)),
_ => throw new ArgumentException(null, nameof(windowHandle)),
};

// The OS might spawn the window on a different monitor to the target parent, so adjustments
// might need to be made because of DPI.
var monitor = _monitorService.GetMonitorFromHandleLocation(windowHandle);
if (MonitorService.HasDpiDifference(monitor, window.Parent))
window.HasPendingDpiAdjustment = true;

return window;
}

private static WindowType GetWindowTypeToCreate(IntPtr windowHandle)
Expand Down
57 changes: 0 additions & 57 deletions GlazeWM.Domain/Windows/CommandHandlers/RunWindowRulesHandler.cs

This file was deleted.

18 changes: 0 additions & 18 deletions GlazeWM.Domain/Windows/Commands/RunWindowRulesCommand.cs

This file was deleted.

0 comments on commit 024f68a

Please sign in to comment.