chore: modify broker related code

[filzrev]

This PR intended to fix part of issue of #2824

What's changed in this PR

1. Add IDisposable interface and remove OnProcessExited event handler

To ensure dispose operation is called synchronously.
Add IDisposable interface.

And modify OnProcessExited callback to dispose DisposeLocalCopyOfClientHandle.
It's required when child process is spawned and exits quickly without BeforeAnythingElse signal.
Without this code, reader.ReadLine(); infinitely waiting, because anonymous pipe is not closed.

2. Add Result enum

Add Result enum to return ProcessDataBlocking result for error logging.

3. Modify Task.Run` code

On current implementation.
Exception that thrown inside ProcessDataBlocking is silently ignored.

Because task is not awaited and TaskScheduler.UnobservedTaskException is not registerd.
So when exception thrown, it cause benchmark freeze at finished.WaitOne line.

This PR move finished.Set code to try-finally block.
To ensure finished.WaitOne is signaled on unexpected exception thrown.

And add GetAwaiter().GetResult() code to wait task completion to throw exception.

4. ProcessDataBlocking code changes

• Modify code to return Result enum.
• Replace else if blocks to if-continue to improve code readability.
• Add null check logic of ReadLine() results when parsing InProcessDiagnoserResults line.
• Add line.StartsWith check to avoid ArgumentOutOfRangeException when getting substring.
  (As far as I've confirmed on original issue's logs. It seems there is cases that incomplete log line is returned from ReadLine())

[adamsitnik]

And modify OnProcessExited callback to dispose DisposeLocalCopyOfClientHandle.
It's required when child process is spawned and exits quickly without BeforeAnythingElse signal.
Without this code, reader.ReadLine(); infinitely waiting, because anonymous pipe is not closed.

If the process exits, the ReadLine should return null as the underlying pipe got closed (when the process exited).

If it's not the case, I can see two other potential issues:

• the parent process has not closed it's copy of the pipe handle (write end in this case)
• the parent process has started another process, that derived the pipe handle and keeps it open until it exits

@filzrev big thanks for taking care of this edge case bug!

[filzrev]

I've removed redundant Task.Run and ManualResetEvent.

It’s remained to preserve the original code behavior. (Run ProcessDataBlocking on worker thread)
But it seems redundant because it block caller thread (Main Thread) in any case.

big thanks for taking care of this edge case bug!

Original code contains OnProcessExited callback already and disposing client handles inside OnProcessExited .

So expected edge case is following

• OnProcessExited asynchronous callback is not called because Process.Dispose is invoked before callback.

[timcassell]

LGTM but I'll let @adamsitnik make the final call.

[filzrev]

It seems PR #2958 including AnonymousPipe to NamedPipe migration.
NamedPipe supporting async operations and more reliable than AnonymousPipe.

So it's not needed if #2958 is expected to be merged in the near future.
In this case following changes are expected o be merged to PR.

1. Run dispose operation synchronously by IDisposable with following reasons
   OnProcessExited handler is called asynchronously and it might cause race conditions.
   OnProcessExited handler is not called when process instance is disposed.
2. null/length check when parsing InProcessDiagnoserResults line (It's expected to be happens pipe is closed during processing)

[timcassell]

It's fine to merge this first. I still have some work to do in that PR, and I can fix the merge conflicts with this.

[timcassell]

The change to named pipe in #2958 is showing freezes on Windows CI just like the issue this is trying to fix. I don't think it's a cause, but it does make it more repro-able. I'm going to go ahead and merge this then update that branch with these changes and see if it helps.