Skip to content

Commit

Permalink
Avoid NullReferenceExeption in GetTextFromQuery (#346)
Browse files Browse the repository at this point in the history
In rare cases, a thread-scheduling race condition can cause a thread's
state to be ThreadState.Stopped even though it hasn't yet executed its
`finally` block, which can result in result.StandardOutput or Error not
being assigned a string, leaving them null. Furthermore, the C# docs say
not to depend on ThreadState. So instead we check for the thread having
ended by seeing whether the `finally` block has assigned a value to its
Results property; until then, Results is null.

This should guarantee the safety of the existing GetTextFromQuery code,
but to be safe we also make sure it can turn null into an empty string.
  • Loading branch information
rmunn authored Aug 30, 2024
1 parent d0b61c6 commit 41c3d97
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/LibChorus/Utilities/HgProcessOutputReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public bool Read(ref Process process, int secondsBeforeTimeOut, IProgress progre
}

//nb: at one point I (jh) tried adding !process.HasExited, but that made things less stable.
while ( _outputReader.ThreadState != ThreadState.Stopped && _errorReader.ThreadState != ThreadState.Stopped)
while (outputReaderArgs.Results == null || errorReaderArgs.Results == null)
{
DateTime end;
lock (this)
Expand Down
6 changes: 3 additions & 3 deletions src/LibChorus/VcsDrivers/Mercurial/HgRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -468,13 +468,13 @@ private string GetTextFromQuery(string query, int secondsBeforeTimeoutOnLocalOpe
{
var result = ExecuteErrorsOk(query, secondsBeforeTimeoutOnLocalOperation);

var standardOutputText = result.StandardOutput.Trim();
var standardOutputText = result.StandardOutput?.Trim();
if (!string.IsNullOrEmpty(standardOutputText))
{
_progress.WriteVerbose(standardOutputText);
}

var standardErrorText = result.StandardError.Trim();
var standardErrorText = result.StandardError?.Trim();
if (!string.IsNullOrEmpty(standardErrorText))
{
_progress.WriteError(standardErrorText);
Expand All @@ -485,7 +485,7 @@ private string GetTextFromQuery(string query, int secondsBeforeTimeoutOnLocalOpe
_progress.WriteWarning("Hg Command {0} left lock", query);
}

return result.StandardOutput;
return result.StandardOutput ?? "";
}

/// <summary>
Expand Down

0 comments on commit 41c3d97

Please sign in to comment.