MICore: Runtime-impactful changes for nullable enablement

gregg-miskelly · Copilot · gregg-miskelly · commit 385b90a5fcf3 · 2026-06-25T17:06:43.000-07:00
- 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 &lt;223556219+Copilot@users.noreply.github.com&gt;
diff --git a/src/MICore/CommandFactories/MICommandFactory.cs b/src/MICore/CommandFactories/MICommandFactory.cs
@@ -48,30 +48,34 @@ public enum AsyncBreakSignal
 
     public abstract class MICommandFactory
     {
-        protected Debugger _debugger;
+        protected readonly Debugger _debugger;
 
         public MIMode Mode { get; private set; }
 
         public abstract string Name { get; }
 
         internal int MajorVersion { get; set; }
 
+        protected MICommandFactory(Debugger debugger)
+        {
+            _debugger = debugger;
+        }
+
         public static MICommandFactory GetInstance(MIMode mode, Debugger debugger)
         {
             MICommandFactory commandFactory;
 
             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
@@ -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
@@ -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
@@ -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();
 
         /// <summary>
         /// The last command we sent over the transport. This includes both the command name and arguments.
         /// </summary>
-        private string _lastCommandText;
+        private string _lastCommandText = string.Empty;
         private uint _lastCommandId;
 
         /// <summary>
@@ -169,6 +169,7 @@ public Debugger(LaunchOptions launchOptions, Logger logger)
             _debuggeePids = new Dictionary<string, int>();
             Logger = logger;
             _miResults = new MIResults(logger);
+            MICommandFactory = MICommandFactory.GetInstance(launchOptions.DebuggerMIMode, this);
         }
 
         protected void SetDebuggerPid(int debuggerPid)
@@ -392,7 +393,7 @@ private async Task<bool> 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<bool> 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<Results> 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<string>()).AsReadOnly(), (_initializationLog?.ToList() ?? new List<string>()).AsReadOnly(), _gdbVersion);
                                 SendUnsupportedWindowsGdbEvent(_gdbVersion);
                             }
                             else
                             {
                                 exception = new MIDebuggerInitializeFailedException(
-                                    this.MICommandFactory.Name, _initialErrors.ToList().AsReadOnly(), _initializationLog.ToList().AsReadOnly());
+                                    this.MICommandFactory.Name, (_initialErrors?.ToList() ?? new List<string>()).AsReadOnly(), (_initializationLog?.ToList() ?? new List<string>()).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
@@ -64,7 +64,6 @@ public DebuggedProcess(bool bLaunched, LaunchOptions launchOptions, ISampleEngin
             _libraryLoaded = new List<string>();
             _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);
 

Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,11 @@ namespace MICore`
`16`	`16`	`{`
`17`	`17`	`internal class LlldbMICommandFactory : MICommandFactory`
`18`	`18`	`{`
	`19`	`+ public LlldbMICommandFactory(Debugger debugger)`
	`20`	`+ : base(debugger)`
	`21`	`+ {`
	`22`	`+ }`
	`23`	`+`
`19`	`24`	`public override string Name`
`20`	`25`	`{`
`21`	`26`	`get { return "LLDB"; }`
Original file line number	Diff line number	Diff line change
`@@ -72,7 +72,7 @@ public bool IsClosed`
`72`	`72`	`public uint MaxInstructionSize { get; private set; }`
`73`	`73`	`public bool Is64BitArch { get; private set; }`
`74`	`74`	`public CommandLock CommandLock { get { return _commandLock; } }`
`75`		`- public MICommandFactory MICommandFactory { get; protected set; }`
	`75`	`+ public MICommandFactory MICommandFactory { get; }`
`76`	`76`	`public Logger Logger { private set; get; }`
`77`	`77`
`78`	`78`	`[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Security", "CA2104:DoNotDeclareReadOnlyMutableReferenceTypes")]`
`@@ -124,12 +124,12 @@ public StoppingEventArgs(Results results, BreakRequest asyncRequest = BreakReque`
`124`	`124`	`}`
`125`	`125`
`126`	`126`	`private ITransport _transport;`
`127`		`- private CommandLock _commandLock = new CommandLock();`
	`127`	`+ private readonly CommandLock _commandLock = new CommandLock();`
`128`	`128`
`129`	`129`	`/// <summary>`
`130`	`130`	`/// The last command we sent over the transport. This includes both the command name and arguments.`
`131`	`131`	`/// </summary>`
`132`		`- private string _lastCommandText;`
	`132`	`+ private string _lastCommandText = string.Empty;`
`133`	`133`	`private uint _lastCommandId;`
`134`	`134`
`135`	`135`	`/// <summary>`
`@@ -169,6 +169,7 @@ public Debugger(LaunchOptions launchOptions, Logger logger)`
`169`	`169`	`_debuggeePids = new Dictionary<string, int>();`
`170`	`170`	`Logger = logger;`
`171`	`171`	`_miResults = new MIResults(logger);`
	`172`	`+ MICommandFactory = MICommandFactory.GetInstance(launchOptions.DebuggerMIMode, this);`
`172`	`173`	`}`
`173`	`174`
`174`	`175`	`protected void SetDebuggerPid(int debuggerPid)`
`@@ -392,7 +393,7 @@ private async Task<bool> DoInternalBreakActions(bool fIsAsyncBreak)`
`392`	`393`	`}`
`393`	`394`	`catch (Exception e) when (ExceptionHelper.BeforeCatch(e, Logger, reportOnlyCorrupting: true))`
`394`	`395`	`{`
`395`		`- if (firstException != null)`
	`396`	`+ if (firstException is null)`
`396`	`397`	`{`
`397`	`398`	`firstException = e;`
`398`	`399`	`}`
`@@ -404,7 +405,7 @@ private async Task<bool> DoInternalBreakActions(bool fIsAsyncBreak)`
`404`	`405`	`{`
`405`	`406`	`if (this.IsClosed)`
`406`	`407`	`{`
`407`		`- source.TrySetException(new DebuggerDisposedException(_closeMessage));`
	`408`	`+ source.TrySetException(new DebuggerDisposedException(GetTargetProcessExitedReason()));`
`408`	`409`	`}`
`409`	`410`	`else`
`410`	`411`	`{`
`@@ -524,7 +525,7 @@ private void Close(string closeMessage)`
`524`	`525`	`Debug.Assert(_closeMessage == null, "Why was Close called more than once? Should be impossible.");`
`525`	`526`
`526`	`527`	`_closeMessage = closeMessage;`
`527`		`- _transport.Close();`
	`528`	`+ _transport?.Close();`
`528`	`529`	`lock (_waitingOperations)`
`529`	`530`	`{`
`530`	`531`	`foreach (var value in _waitingOperations.Values)`
`@@ -905,7 +906,7 @@ private Task<Results> CmdAsyncInternal(string command, ResultClass expectedResul`
`905`	`906`	`{`
`906`	`907`	`if (this.IsClosed)`
`907`	`908`	`{`
`908`		`- throw new DebuggerDisposedException(_closeMessage);`
	`909`	`+ throw new DebuggerDisposedException(GetTargetProcessExitedReason());`
`909`	`910`	`}`
`910`	`911`
`911`	`912`	`id = ++_lastCommandId;`
`@@ -992,13 +993,13 @@ public void OnDebuggerProcessExit(/OPTIONAL/ string exitCode)`
`992`	`993`	`if (isMinGWOrCygwin && IsUnsupportedWindowsGdbVersion(_gdbVersion))`
`993`	`994`	`{`
`994`	`995`	`exception = new MIDebuggerInitializeFailedUnsupportedGdbException(`
`995`		`- this.MICommandFactory.Name, _initialErrors.ToList().AsReadOnly(), _initializationLog.ToList().AsReadOnly(), _gdbVersion);`
	`996`	`+ this.MICommandFactory.Name, (_initialErrors?.ToList() ?? new List<string>()).AsReadOnly(), (_initializationLog?.ToList() ?? new List<string>()).AsReadOnly(), _gdbVersion);`
`996`	`997`	`SendUnsupportedWindowsGdbEvent(_gdbVersion);`
`997`	`998`	`}`
`998`	`999`	`else`
`999`	`1000`	`{`
`1000`	`1001`	`exception = new MIDebuggerInitializeFailedException(`
`1001`		`- this.MICommandFactory.Name, _initialErrors.ToList().AsReadOnly(), _initializationLog.ToList().AsReadOnly());`
	`1002`	`+ this.MICommandFactory.Name, (_initialErrors?.ToList() ?? new List<string>()).AsReadOnly(), (_initializationLog?.ToList() ?? new List<string>()).AsReadOnly());`
`1002`	`1003`	`}`
`1003`	`1004`
`1004`	`1005`	`_initialErrors = null;`
`@@ -1029,7 +1030,7 @@ public void OnDebuggerProcessExit(/OPTIONAL/ string exitCode)`
`1029`	`1030`	`{`
`1030`	`1031`	`if (DebuggerExitEvent != null)`
`1031`	`1032`	`{`
`1032`		`- DebuggerExitEvent(this, null);`
	`1033`	`+ DebuggerExitEvent(this, EventArgs.Empty);`
`1033`	`1034`	`}`
`1034`	`1035`	`}`
`1035`	`1036`	`}`
`@@ -1419,8 +1420,7 @@ this.LaunchOptions is LocalLaunchOptions &&`
`1419`	`1420`
`1420`	`1421`	`private void OnNotificationOutput(string cmd)`
`1421`	`1422`	`{`
`1422`		`- Results results = null;`
`1423`		`- if ((results = MICommandFactory.IsModuleLoad(cmd)) != null)`
	`1423`	`+ if (MICommandFactory.IsModuleLoad(cmd) is Results results)`
`1424`	`1424`	`{`
`1425`	`1425`	`if (LibraryLoadEvent != null)`
`1426`	`1426`	`{`
`@@ -1460,12 +1460,12 @@ private void OnNotificationOutput(string cmd)`
`1460`	`1460`	`else if (cmd.StartsWith("thread-created,", StringComparison.Ordinal))`
`1461`	`1461`	`{`
`1462`	`1462`	`results = _miResults.ParseResultList(cmd.Substring("thread-created,".Length));`
`1463`		`- ThreadCreatedEvent(this, new ResultEventArgs(results, 0));`
	`1463`	`+ ThreadCreatedEvent?.Invoke(this, new ResultEventArgs(results, 0));`
`1464`	`1464`	`}`
`1465`	`1465`	`else if (cmd.StartsWith("thread-exited,", StringComparison.Ordinal))`
`1466`	`1466`	`{`
`1467`	`1467`	`results = _miResults.ParseResultList(cmd.Substring("thread-exited,".Length));`
`1468`		`- ThreadExitedEvent(this, new ResultEventArgs(results, 0));`
	`1468`	`+ ThreadExitedEvent?.Invoke(this, new ResultEventArgs(results, 0));`
`1469`	`1469`	`}`
`1470`	`1470`	`else if (cmd.StartsWith("telemetry,", StringComparison.Ordinal))`
`1471`	`1471`	`{`
`@@ -1619,14 +1619,15 @@ private async void PostCommand(string cmd)`
`1619`	`1619`
`1620`	`1620`	`private void SendToTransport(string cmd)`
`1621`	`1621`	`{`
`1622`		`- _transport.Send(cmd);`
	`1622`	`+ ITransport transport = _transport ?? throw new InvalidOperationException();`
	`1623`	`+ transport.Send(cmd);`
`1623`	`1624`
`1624`	`1625`	`// https://github.com/Microsoft/MIEngine/issues/616 :`
`1625`	`1626`	`// If it is local gdb (MinGW/Cygwin) on Windows, we need to send an extra line after commands`
`1626`	`1627`	`// so that if it errors, the error will come through.`
`1627`	`1628`	`if (this.SendNewLineAfterCmd)`
`1628`	`1629`	`{`
`1629`		`- _transport.Send(String.Empty);`
	`1630`	`+ transport.Send(String.Empty);`
`1630`	`1631`	`}`
`1631`	`1632`	`}`
`1632`	`1633`