fix: Keyboard lost key-up bug

Author: maximilien-noal

src/Spice86.Core/Emulator/Devices/Input/Keyboard/PS2Keyboard.cs

@@ -8,6 +8,7 @@ namespace Spice86.Core.Emulator.Devices.Input.Keyboard;
 
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Linq;
 
 /// <summary>
 /// Emulates a PS/2 keyboard device, handling key events, scancode translation, and communication with a keyboard
@@ -33,6 +34,14 @@ public partial class PS2Keyboard {
     private RepeatData _repeat = new() { Key = PcKeyboardKey.None, WaitMs = 0, PauseMs = 500, RateMs = 33 };
     private readonly Dictionary<byte, Set3CodeInfoEntry> _set3CodeInfo = new();
 
+    /// <summary>
+    /// Keys the device currently believes are held down. A host key-up can be lost (window lost focus,
+    /// input dropped while paused, frame-buffer overflow) and the guest only ever clears a modifier on its
+    /// BREAK scancode, so this set is the bookkeeping that lets the device reconcile and release what it
+    /// still thinks is down.
+    /// </summary>
+    private readonly HashSet<PcKeyboardKey> _pressedKeys = new();
+
     /// <summary>
     /// State of keyboard LEDs, as requested via keyboard controller
     /// </summary>
@@ -130,6 +139,11 @@ private void EnqueueScanCodeFrame(List<byte>? scanCode) {
         if (_keyboardFrameBuffer.Count == BufferSizeInScancodes) {
             _keyboardFrameBuffer.Clear();
             _bufferOverflowed = true;
+            // A discarded key-up would otherwise latch that key (and its modifier bits) in the guest forever.
+            // Reconcile the held-key set so no key is believed to be down any more.
+            _pressedKeys.Clear();
+            _repeat.Key = PcKeyboardKey.None;
+            _repeat.WaitMs = 0;
             return;
         }
 
@@ -521,6 +535,44 @@ internal void NotifyReadyForFrame() {
         TransferNextBufferedFrameIfReady();
     }
 
+    /// <summary>
+    /// Releases every key the device still believes is down, emitting a proper BREAK frame for each one,
+    /// and stops any in-flight typematic repeat.
+    /// </summary>
+    /// <remarks>
+    /// Call this when the host's key state can no longer be trusted: the window lost activation, the
+    /// emulator was paused, or the keyboard frame buffer overflowed and discarded a key-up. The guest only
+    /// clears a modifier on its BREAK scancode, so a lost key-up otherwise latches it forever (e.g. Alt
+    /// left in <c>KB_FLAG</c>), silently changing how every other key is translated. This method is safe to
+    /// call on the emulator thread only; marshal cross-thread calls onto the emulator thread first.
+    /// </remarks>
+    public void ReleaseAllKeys() {
+        if (_pressedKeys.Count == 0) {
+            _repeat.Key = PcKeyboardKey.None;
+            _repeat.WaitMs = 0;
+            return;
+        }
+
+        // Emit a BREAK frame for each held key in a deterministic order so the guest sees a stable sequence.
+        List<PcKeyboardKey> heldKeys = _pressedKeys.OrderBy(key => key).ToList();
+        foreach (PcKeyboardKey key in heldKeys) {
+            EnqueueScanCodeFrame(GetReleaseScanCode(key));
+        }
+
+        _pressedKeys.Clear();
+        _repeat.Key = PcKeyboardKey.None;
+        _repeat.WaitMs = 0;
+    }
+
+    private List<byte> GetReleaseScanCode(PcKeyboardKey keyType) {
+        return _codeSet switch {
+            1 => _scancodeConverter.GetScanCode1(keyType, isPressed: false),
+            2 => _scancodeConverter.GetScanCode2(keyType, isPressed: false),
+            3 => _scancodeConverter.GetScanCode3(keyType, isPressed: false),
+            _ => _scancodeConverter.GetScanCode1(keyType, isPressed: false),
+        };
+    }
+
     /// <summary>
     /// Adds a key event to be processed by the keyboard.
     /// </summary>
@@ -531,6 +583,16 @@ public void EnqueueKeyEvent(PcKeyboardKey keyType, bool isPressed) {
             return;
         }
 
+        if (keyType is PcKeyboardKey.None or PcKeyboardKey.Pause or PcKeyboardKey.PrintScreen) {
+            // Keys that are not tracked as held: Pause has no BREAK frame, PrintScreen is a momentary
+            // key-chord, and None is an unsupported key. Keeping them out of the held-key set means a
+            // later release-all does not try to emit a BREAK scancode that does not exist.
+        } else if (isPressed) {
+            _pressedKeys.Add(keyType);
+        } else {
+            _pressedKeys.Remove(keyType);
+        }
+
         List<byte> scanCode = new();
 
         switch (_codeSet) {

src/Spice86.Core/Emulator/Mcp/EmulatorMcpServices.cs

@@ -70,7 +70,17 @@ public sealed class EmulatorMcpServices(
     /// <summary>
     /// Gets or sets the Intel 8042 PS/2 controller used for keyboard/mouse MCP automation.
     /// </summary>
-    public Intel8042Controller? Intel8042Controller { get; set; }
+    public Intel8042Controller? Intel8042Controller {
+        get => _intel8042Controller;
+        set {
+            _intel8042Controller = value;
+            if (value is not null) {
+                SubscribeKeyboardReconcileOnPause();
+            }
+        }
+    }
+
+    private Intel8042Controller? _intel8042Controller;
 
     /// <summary>
     /// Gets or sets the input event hub. MCP tools must post keyboard/mouse
@@ -124,6 +134,26 @@ public sealed class EmulatorMcpServices(
     /// </summary>
     public Dos? Dos { get; set; }
 
+    private bool _keyboardReconcileSubscribed;
+
+    /// <summary>
+    /// Subscribes the keyboard device to release all held keys whenever the emulator pauses. Key-ups
+    /// dropped while paused (or lost when the window deactivates) would otherwise leave a modifier
+    /// latched in the guest's BIOS state. The release is marshalled onto the emulator thread via
+    /// <see cref="InputEventHub"/> so it is safe to call from the UI thread.
+    /// </summary>
+    internal void SubscribeKeyboardReconcileOnPause() {
+        if (_keyboardReconcileSubscribed
+            || Intel8042Controller is null
+            || InputEventHub is null) {
+            return;
+        }
+
+        InputEventHub inputEventHub = InputEventHub;
+        PauseHandler.Pausing += () => inputEventHub.ReleaseAllKeys();
+        _keyboardReconcileSubscribed = true;
+    }
+
     // Shared MCP breakpoint tracking state (survives transient tool instances)
     private readonly object _mcpBreakpointsLock = new();
     private readonly Lock _toolsLock = new();

src/Spice86.Core/Emulator/VM/InputEventQueue.cs

@@ -59,6 +59,20 @@ private void Enqueue(Action action) {
     /// </summary>
     public void PostToEmulatorThread(Action action) => Enqueue(action);
 
+    /// <summary>
+    /// Thread-safe request to release every key the keyboard device still believes is down, emitting a
+    /// proper BREAK frame for each one. Call this when the host's key state can no longer be trusted
+    /// (e.g. the window lost activation and a key-up was never delivered), so the guest does not keep a
+    /// modifier latched. The release is enqueued and runs on the emulator thread with the rest of the input.
+    /// </summary>
+    public void ReleaseAllKeys() =>
+        Enqueue(() => KeyboardDevice?.ReleaseAllKeys());
+
+    /// <summary>
+    /// The PS/2 keyboard device whose held-key state is reconciled by <see cref="ReleaseAllKeys"/>.
+    /// </summary>
+    internal PS2Keyboard? KeyboardDevice { get; set; }
+
     /// <summary>
     /// Enqueues a keyboard event to be fired on the emulator thread, following
     /// the same path as UI keyboard events (PhysicalKey → PS2Keyboard scancode pipeline).

src/Spice86/Spice86DependencyInjection.cs

@@ -562,6 +562,7 @@ internal Spice86DependencyInjection(Configuration configuration, MainWindow? mai
             configuration.FailOnUnhandledPort, loggerService, inputEventHub);
         emulatorMcpServices.Intel8042Controller = intel8042Controller;
         emulatorMcpServices.BiosKeyboardBuffer = biosKeyboardBuffer;
+        inputEventHub.KeyboardDevice = intel8042Controller.KeyboardDevice;
         BiosKeyboardInt9Handler biosKeyboardInt9Handler = new(memory, biosDataArea,
             stack, state, cfgCpu, dualPic, systemBiosInt15Handler,
             intel8042Controller, biosKeyboardBuffer, loggerService);
@@ -795,6 +796,7 @@ internal Spice86DependencyInjection(Configuration configuration, MainWindow? mai
 
         if (mainWindow != null && uiDispatcher != null &&
             hostStorageProvider != null && textClipboard != null && mainWindowViewModel != null) {
+            mainWindowViewModel.OnWindowDeactivated = () => inputEventHub.ReleaseAllKeys();
             IMessenger messenger = WeakReferenceMessenger.Default;
 
             BreakpointsTabPlugin breakpointsTabPlugin = new(state, pauseHandler, messenger,

src/Spice86/ViewModels/MainWindowViewModel.cs

@@ -106,6 +106,13 @@ public double? TimeMultiplier {
     /// <summary>Fired when this view model is being disposed so the composition root can tear down.</summary>
     internal event Action? Disposing;
 
+    /// <summary>
+    /// Thread-safe action that releases every key the keyboard device still believes is down. Invoked when
+    /// the window loses activation, so a key-up swallowed by the shell (e.g. Alt+Tab) does not latch a
+    /// modifier in the guest. Assigned by the composition root; null in headless mode.
+    /// </summary>
+    public Action? OnWindowDeactivated { get; set; }
+
     public MainWindowViewModel(
         EmulatorDisplayViewModel display,
         EmulatorSession session,

src/Spice86/Views/MainWindow.axaml.cs

@@ -162,6 +162,9 @@ private void OnPointerCaptureLost(object? sender, PointerCaptureLostEventArgs e)
 
     private void OnWindowDeactivated(object? sender, EventArgs e) {
         ReleaseCapture();
+        // The shell may swallow key-ups (e.g. Alt+Tab) while the window is inactive. Release any key the
+        // device still believes is down so the guest does not keep a modifier latched.
+        (DataContext as MainWindowViewModel)?.OnWindowDeactivated?.Invoke();
     }
 
     private void OnWindowPositionChanged(object? sender, PixelPointEventArgs e) {