videre-project / MTGOSDK

A software development kit (SDK) for inspecting and interacting with the Magic: The Gathering Online (MTGO) client.
Apache License 2.0
9 stars 1 forks source link

[third_party/RemoteNET] ScubaDiver: Fix Win32Error traced from disposing WindowsProcessDataReader #3

Closed Qonfused closed 1 year ago

Qonfused commented 1 year ago

The WindowsProcessDataReader's dispose method attempts to kill the snapshot process without waiting for process exit. As a result, a Win32Exception is thrown (as we don't have the correct privileges to force an exit without yielding first) which will repeatedly spam any subscribed Trace listeners (as a Trace.Write is called internally).

This seems to be called when invoking either the DataTarget.Dispose() or ClrRuntime.Dispose() methods. You can observe this behavior in Loupe Desktop when injecting RemoteNET into the MTGO client, which will trace this error on Diver startup and when pinning objects, etc.

Unfortunately, we can't (and shouldn't) unsubscribe Trace listeners as this may negatively affect Gibraltar logging on the MTGO client, which makes extensive use of Trace.Write calls to inform state changes.

This is a regression from https://github.com/microsoft/clrmd/pull/926 that we'll need to work around by reimplementing the dispose method w/ the use of reflection.

### Tasks
- [x] Kill and yield to snapshot process exit
- [x] Free snapshot process/handle memory
- [x] Close native process handle
- [x] Mark DataReader class as disposed
Qonfused commented 1 year ago

Note that this workaround requires disposing of the DataTarget instance before the ClrRuntime, as the ClrRuntime will call each helper class's Dispose methods. This workaround sets the _dispose property to True to mark the class as disposed, which skips the subsequent calls that would call the problematic Dispose() method.

Edit: Refer to commits 6170685..cd81390 for the applied changes.