From 1d87c2ace3df7765d10ad7ce75dc5dbec6c20642 Mon Sep 17 00:00:00 2001 From: Gregg Miskelly Date: Thu, 25 Jun 2026 14:08:25 -0700 Subject: [PATCH 1/2] DebugEngineHost nullability fixes --- src/DebugEngineHost.Common/HostLogChannel.cs | 6 +++--- src/DebugEngineHost.Stub/DebugEngineHost.ref.cs | 4 ++-- src/DebugEngineHost.VSCode/HostLogger.cs | 2 +- src/DebugEngineHost.VSCode/HostRunInTerminal.cs | 8 ++++---- src/DebugEngineHost/HostLogger.cs | 2 +- src/DebugEngineHost/HostRunInTerminal.cs | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/DebugEngineHost.Common/HostLogChannel.cs b/src/DebugEngineHost.Common/HostLogChannel.cs index 699cdb3ad..5f4434193 100644 --- a/src/DebugEngineHost.Common/HostLogChannel.cs +++ b/src/DebugEngineHost.Common/HostLogChannel.cs @@ -50,15 +50,15 @@ public interface ILogChannel public class HostLogChannel : ILogChannel { - private readonly Action _log; + private readonly Action? _log; private StreamWriter? _logFile; private LogLevel _minLevelToBeLogged; private readonly object _lock = new object(); - private HostLogChannel() { _log = null!; } + private HostLogChannel() { } - public HostLogChannel(Action logAction, string? file, LogLevel logLevel) + public HostLogChannel(Action? logAction, string? file, LogLevel logLevel) { _log = logAction; diff --git a/src/DebugEngineHost.Stub/DebugEngineHost.ref.cs b/src/DebugEngineHost.Stub/DebugEngineHost.ref.cs index 7b7fae658..c34e817e5 100644 --- a/src/DebugEngineHost.Stub/DebugEngineHost.ref.cs +++ b/src/DebugEngineHost.Stub/DebugEngineHost.ref.cs @@ -259,7 +259,7 @@ public static class HostLogger /// /// The callback to use to send the engine log. /// The level of the log to filter the channel on. - public static void EnableHostLogging(Action callback, LogLevel level = LogLevel.Verbose) + public static void EnableHostLogging(Action? callback, LogLevel level = LogLevel.Verbose) { throw new NotImplementedException(); } @@ -527,7 +527,7 @@ public static bool IsRunInTerminalAvailable() /// true if the message is sent, false if not. [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "cwd")] [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1006:DoNotNestGenericTypesInMemberSignatures")] - public static void RunInTerminal(string title, string workingDirectory, bool useExternalConsole, IReadOnlyList commandArgs, IReadOnlyDictionary environmentVariables, Action success, Action failure) + public static void RunInTerminal(string title, string workingDirectory, bool useExternalConsole, IReadOnlyList commandArgs, IReadOnlyDictionary environmentVariables, Action success, Action failure) { throw new NotImplementedException(); } diff --git a/src/DebugEngineHost.VSCode/HostLogger.cs b/src/DebugEngineHost.VSCode/HostLogger.cs index 9d784538e..75190588c 100644 --- a/src/DebugEngineHost.VSCode/HostLogger.cs +++ b/src/DebugEngineHost.VSCode/HostLogger.cs @@ -21,7 +21,7 @@ public static void EnableNatvisDiagnostics(Action callback, LogLevel lev } } - public static void EnableHostLogging(Action callback, LogLevel level = LogLevel.Verbose) + public static void EnableHostLogging(Action? callback, LogLevel level = LogLevel.Verbose) { if (s_engineLogChannel is null) { diff --git a/src/DebugEngineHost.VSCode/HostRunInTerminal.cs b/src/DebugEngineHost.VSCode/HostRunInTerminal.cs index 60dbcf109..62faf45de 100644 --- a/src/DebugEngineHost.VSCode/HostRunInTerminal.cs +++ b/src/DebugEngineHost.VSCode/HostRunInTerminal.cs @@ -9,7 +9,7 @@ namespace Microsoft.DebugEngineHost { public static class HostRunInTerminal { - private static Action, Dictionary, Action, Action>? s_runInTerminalCallback; + private static Action, Dictionary, Action, Action>? s_runInTerminalCallback; /// /// Checks to see if RunInTerminal is available @@ -23,11 +23,11 @@ public static bool IsRunInTerminalAvailable() /// /// Passes the call to the UI to attempt to RunInTerminal if possible. /// - public static void RunInTerminal(string title, string cwd, bool useExternalConsole, IReadOnlyList commandArgs, IReadOnlyDictionary environmentVars, Action success, Action failure) + public static void RunInTerminal(string title, string cwd, bool useExternalConsole, IReadOnlyList commandArgs, IReadOnlyDictionary environmentVars, Action success, Action failure) { if (s_runInTerminalCallback is not null) { - Dictionary env = new Dictionary(); + Dictionary env = new Dictionary(); foreach (var item in environmentVars) { env.Add(item.Key, item.Value); @@ -41,7 +41,7 @@ public static void RunInTerminal(string title, string cwd, bool useExternalConso /// Registers callback to call when RunInTerminal is called /// /// Callback for RunInTerminal - public static void RegisterRunInTerminalCallback(Action, Dictionary, Action, Action> runInTerminalCallback) + public static void RegisterRunInTerminalCallback(Action, Dictionary, Action, Action> runInTerminalCallback) { Debug.Assert(runInTerminalCallback != null, "Callback should not be null."); s_runInTerminalCallback = runInTerminalCallback; diff --git a/src/DebugEngineHost/HostLogger.cs b/src/DebugEngineHost/HostLogger.cs index ab29a977f..271fcf718 100644 --- a/src/DebugEngineHost/HostLogger.cs +++ b/src/DebugEngineHost/HostLogger.cs @@ -19,7 +19,7 @@ public static class HostLogger private static FeedbackLogBuffer? s_circularBuffer; private static VSFeedbackLogger? s_feedbackLogger; - public static void EnableHostLogging(Action callback, LogLevel level = LogLevel.Verbose) + public static void EnableHostLogging(Action? callback, LogLevel level = LogLevel.Verbose) { if (s_engineLogChannel is null) { diff --git a/src/DebugEngineHost/HostRunInTerminal.cs b/src/DebugEngineHost/HostRunInTerminal.cs index 05079fdd1..243857d8d 100644 --- a/src/DebugEngineHost/HostRunInTerminal.cs +++ b/src/DebugEngineHost/HostRunInTerminal.cs @@ -22,7 +22,7 @@ public static bool IsRunInTerminalAvailable() [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "cwd")] [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1006:DoNotNestGenericTypesInMemberSignatures")] - public static void RunInTerminal(string title, string workingDirectory, bool useExternalConsole, IReadOnlyList commandArgs, IReadOnlyDictionary environmentVariables, Action success, Action failure) + public static void RunInTerminal(string title, string workingDirectory, bool useExternalConsole, IReadOnlyList commandArgs, IReadOnlyDictionary environmentVariables, Action success, Action failure) { throw new NotImplementedException(); } From 385b90a5fcf3430f165170e2a0650120c969c060 Mon Sep 17 00:00:00 2001 From: Gregg Miskelly Date: Thu, 25 Jun 2026 14:34:48 -0700 Subject: [PATCH 2/2] MICore: Runtime-impactful changes for nullable enablement - Refactor MICommandFactory to use constructor injection (readonly _debugger field) - Move MICommandFactory.GetInstance() from DebuggedProcess to Debugger constructor - Fix bug: firstException should only be set when null (was checking != null) - Add null safety: _transport?.Close(), ThreadCreatedEvent?.Invoke, ThreadExitedEvent?.Invoke - Add null throw in SendToTransport for null transport - Use GetTargetProcessExitedReason() instead of raw _closeMessage - Null-safe access to _initialErrors/_initializationLog in OnDebuggerProcessExit - Pass EventArgs.Empty instead of null for DebuggerExitEvent - Use pattern matching for IsModuleLoad check - Make _commandLock readonly, initialize _lastCommandText Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CommandFactories/MICommandFactory.cs | 12 ++++--- src/MICore/CommandFactories/gdb.cs | 5 +++ src/MICore/CommandFactories/lldb.cs | 5 +++ src/MICore/Debugger.cs | 33 ++++++++++--------- .../Engine.Impl/DebuggedProcess.cs | 1 - 5 files changed, 35 insertions(+), 21 deletions(-) diff --git a/src/MICore/CommandFactories/MICommandFactory.cs b/src/MICore/CommandFactories/MICommandFactory.cs index 07de24092..a5d0f01ce 100644 --- a/src/MICore/CommandFactories/MICommandFactory.cs +++ b/src/MICore/CommandFactories/MICommandFactory.cs @@ -48,7 +48,7 @@ public enum AsyncBreakSignal public abstract class MICommandFactory { - protected Debugger _debugger; + protected readonly Debugger _debugger; public MIMode Mode { get; private set; } @@ -56,6 +56,11 @@ public abstract class MICommandFactory internal int MajorVersion { get; set; } + protected MICommandFactory(Debugger debugger) + { + _debugger = debugger; + } + public static MICommandFactory GetInstance(MIMode mode, Debugger debugger) { MICommandFactory commandFactory; @@ -63,15 +68,14 @@ public static MICommandFactory GetInstance(MIMode mode, Debugger debugger) switch (mode) { case MIMode.Gdb: - commandFactory = new GdbMICommandFactory(); + commandFactory = new GdbMICommandFactory(debugger); break; case MIMode.Lldb: - commandFactory = new LlldbMICommandFactory(); + commandFactory = new LlldbMICommandFactory(debugger); break; default: throw new ArgumentException(null, nameof(mode)); } - commandFactory._debugger = debugger; commandFactory.Mode = mode; commandFactory.Radix = 10; return commandFactory; diff --git a/src/MICore/CommandFactories/gdb.cs b/src/MICore/CommandFactories/gdb.cs index 83100ab3e..60954d46a 100644 --- a/src/MICore/CommandFactories/gdb.cs +++ b/src/MICore/CommandFactories/gdb.cs @@ -19,6 +19,11 @@ internal class GdbMICommandFactory : MICommandFactory private int _currentThreadId = 0; private uint _currentFrameLevel = 0; + public GdbMICommandFactory(Debugger debugger) + : base(debugger) + { + } + public override string Name { get { return "GDB"; } diff --git a/src/MICore/CommandFactories/lldb.cs b/src/MICore/CommandFactories/lldb.cs index b430f3257..f423c2fc0 100644 --- a/src/MICore/CommandFactories/lldb.cs +++ b/src/MICore/CommandFactories/lldb.cs @@ -16,6 +16,11 @@ namespace MICore { internal class LlldbMICommandFactory : MICommandFactory { + public LlldbMICommandFactory(Debugger debugger) + : base(debugger) + { + } + public override string Name { get { return "LLDB"; } diff --git a/src/MICore/Debugger.cs b/src/MICore/Debugger.cs index 710d6d70c..b3a225497 100755 --- a/src/MICore/Debugger.cs +++ b/src/MICore/Debugger.cs @@ -72,7 +72,7 @@ public bool IsClosed public uint MaxInstructionSize { get; private set; } public bool Is64BitArch { get; private set; } public CommandLock CommandLock { get { return _commandLock; } } - public MICommandFactory MICommandFactory { get; protected set; } + public MICommandFactory MICommandFactory { get; } public Logger Logger { private set; get; } [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Security", "CA2104:DoNotDeclareReadOnlyMutableReferenceTypes")] @@ -124,12 +124,12 @@ public StoppingEventArgs(Results results, BreakRequest asyncRequest = BreakReque } private ITransport _transport; - private CommandLock _commandLock = new CommandLock(); + private readonly CommandLock _commandLock = new CommandLock(); /// /// The last command we sent over the transport. This includes both the command name and arguments. /// - private string _lastCommandText; + private string _lastCommandText = string.Empty; private uint _lastCommandId; /// @@ -169,6 +169,7 @@ public Debugger(LaunchOptions launchOptions, Logger logger) _debuggeePids = new Dictionary(); Logger = logger; _miResults = new MIResults(logger); + MICommandFactory = MICommandFactory.GetInstance(launchOptions.DebuggerMIMode, this); } protected void SetDebuggerPid(int debuggerPid) @@ -392,7 +393,7 @@ private async Task DoInternalBreakActions(bool fIsAsyncBreak) } catch (Exception e) when (ExceptionHelper.BeforeCatch(e, Logger, reportOnlyCorrupting: true)) { - if (firstException != null) + if (firstException is null) { firstException = e; } @@ -404,7 +405,7 @@ private async Task DoInternalBreakActions(bool fIsAsyncBreak) { if (this.IsClosed) { - source.TrySetException(new DebuggerDisposedException(_closeMessage)); + source.TrySetException(new DebuggerDisposedException(GetTargetProcessExitedReason())); } else { @@ -524,7 +525,7 @@ private void Close(string closeMessage) Debug.Assert(_closeMessage == null, "Why was Close called more than once? Should be impossible."); _closeMessage = closeMessage; - _transport.Close(); + _transport?.Close(); lock (_waitingOperations) { foreach (var value in _waitingOperations.Values) @@ -905,7 +906,7 @@ private Task CmdAsyncInternal(string command, ResultClass expectedResul { if (this.IsClosed) { - throw new DebuggerDisposedException(_closeMessage); + throw new DebuggerDisposedException(GetTargetProcessExitedReason()); } id = ++_lastCommandId; @@ -992,13 +993,13 @@ public void OnDebuggerProcessExit(/*OPTIONAL*/ string exitCode) if (isMinGWOrCygwin && IsUnsupportedWindowsGdbVersion(_gdbVersion)) { exception = new MIDebuggerInitializeFailedUnsupportedGdbException( - this.MICommandFactory.Name, _initialErrors.ToList().AsReadOnly(), _initializationLog.ToList().AsReadOnly(), _gdbVersion); + this.MICommandFactory.Name, (_initialErrors?.ToList() ?? new List()).AsReadOnly(), (_initializationLog?.ToList() ?? new List()).AsReadOnly(), _gdbVersion); SendUnsupportedWindowsGdbEvent(_gdbVersion); } else { exception = new MIDebuggerInitializeFailedException( - this.MICommandFactory.Name, _initialErrors.ToList().AsReadOnly(), _initializationLog.ToList().AsReadOnly()); + this.MICommandFactory.Name, (_initialErrors?.ToList() ?? new List()).AsReadOnly(), (_initializationLog?.ToList() ?? new List()).AsReadOnly()); } _initialErrors = null; @@ -1029,7 +1030,7 @@ public void OnDebuggerProcessExit(/*OPTIONAL*/ string exitCode) { if (DebuggerExitEvent != null) { - DebuggerExitEvent(this, null); + DebuggerExitEvent(this, EventArgs.Empty); } } } @@ -1419,8 +1420,7 @@ this.LaunchOptions is LocalLaunchOptions && private void OnNotificationOutput(string cmd) { - Results results = null; - if ((results = MICommandFactory.IsModuleLoad(cmd)) != null) + if (MICommandFactory.IsModuleLoad(cmd) is Results results) { if (LibraryLoadEvent != null) { @@ -1460,12 +1460,12 @@ private void OnNotificationOutput(string cmd) else if (cmd.StartsWith("thread-created,", StringComparison.Ordinal)) { results = _miResults.ParseResultList(cmd.Substring("thread-created,".Length)); - ThreadCreatedEvent(this, new ResultEventArgs(results, 0)); + ThreadCreatedEvent?.Invoke(this, new ResultEventArgs(results, 0)); } else if (cmd.StartsWith("thread-exited,", StringComparison.Ordinal)) { results = _miResults.ParseResultList(cmd.Substring("thread-exited,".Length)); - ThreadExitedEvent(this, new ResultEventArgs(results, 0)); + ThreadExitedEvent?.Invoke(this, new ResultEventArgs(results, 0)); } else if (cmd.StartsWith("telemetry,", StringComparison.Ordinal)) { @@ -1619,14 +1619,15 @@ private async void PostCommand(string cmd) private void SendToTransport(string cmd) { - _transport.Send(cmd); + ITransport transport = _transport ?? throw new InvalidOperationException(); + transport.Send(cmd); // https://github.com/Microsoft/MIEngine/issues/616 : // If it is local gdb (MinGW/Cygwin) on Windows, we need to send an extra line after commands // so that if it errors, the error will come through. if (this.SendNewLineAfterCmd) { - _transport.Send(String.Empty); + transport.Send(String.Empty); } } diff --git a/src/MIDebugEngine/Engine.Impl/DebuggedProcess.cs b/src/MIDebugEngine/Engine.Impl/DebuggedProcess.cs index 804709bc2..f11ff57d6 100755 --- a/src/MIDebugEngine/Engine.Impl/DebuggedProcess.cs +++ b/src/MIDebugEngine/Engine.Impl/DebuggedProcess.cs @@ -64,7 +64,6 @@ public DebuggedProcess(bool bLaunched, LaunchOptions launchOptions, ISampleEngin _libraryLoaded = new List(); _loadOrder = 0; _deleteEntryPointBreakpoint = false; - MICommandFactory = MICommandFactory.GetInstance(launchOptions.DebuggerMIMode, this); _waitDialog = (MICommandFactory.SupportsStopOnDynamicLibLoad() && launchOptions.WaitDynamicLibLoad) ? new HostWaitDialog(ResourceStrings.LoadingSymbolMessage, ResourceStrings.LoadingSymbolCaption) : null; Natvis = new Natvis.Natvis(this, launchOptions.ShowDisplayString, configStore);