Include list of processes that have the file open in IOException message on Windows

[tmat]

Description

Windows's Restart Manage allows to query for process that have a file open:
https://learn.microsoft.com/en-us/windows/win32/api/restartmanager/nf-restartmanager-rmgetlist

This Win32 API is used by msbuild and Roslyn to provide better error message to users, when a build task or the compiler attempts to write to a file that's locked by another process. Having this info in the error message makes it easier to diagnose intermittent file locking issues (e.g. on CI).

The proposal is to move this code to runtime and automatically list process names and ids in the message of System.IO.IOException (The process cannot access the file ... because it is being used by another process.). Possibly allow controlling it by an AppContext switch, so that a host app could decide whether or not to include this information in all instances of IOException caused by access to a file used by another process.

Reproduction Steps

n/a

Expected behavior

The message of System.IO.IOException is
The process cannot access the file ... because it is being used by another process. The file may be locked by <app name> (<pid>), <app name> (<pid>), ...

Actual behavior

The message of System.IO.IOException does not include process information:
The process cannot access the file ... because it is being used by another process.

Regression?

no

Known Workarounds

No response

Configuration

No response

Other information

No response

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

[huoyaoyuan]

Does it need an additional syscall that has potential race condition issue?
Is there any security concern?

[KalleOlaviNiemitalo]

Yes, it needs additional syscalls. The process that caused the error may have closed the file already; additional processes may have opened the file after the error; and some of the listed processes may have opened the file with file sharing flags that did not conflict with the access that failed.

I'm not sure whether Restart Manager works with remote files (UNC paths).

Regarding security concerns: applications can already call the Restart Manager API directly so this doesn't let applications do anything new.

Risks:

• Disclosing process IDs and names in Exception.Message: An application might then further disclose Exception.Message to untrusted users, e.g. in an HTTP response. ASP.NET documentation has advised against this (How to: Display Safe Error Messages), though. The process IDs don't seem very useful to attackers either, except perhaps for detecting that a process crashed and was restarted. Process names might reveal that vulnerable software is being used, especially if the file name includes a version number.
• Performance: Looking up the process IDs and names incurs a cost of time and memory. An AppContext switch would mitigate this.
• Duplicate output if an application is already using Restart Manager. This is a reason against backporting the feature to older releases of .NET.
• Localization: An application may recognize IOException.HResult and display an error message in a language for which .NET Runtime does not have string resources. If the list of processes is only available from Exception.Message, then it will be difficult to parse from there and reformat into the localized message. For this use case, it would be handy if the list were available in a more structured way as well.

[tmat]

All valid points. Hence the suggestion to

allow controlling it by an AppContext switch, so that a host app could decide whether or not to include this information in all instances of IOException caused by access to a file used by another process.

Also, it is understood that the information might not be precise. The error message Roslyn displays is "it may be locked by MyApp (12345)".

[tmat]

NuGet could use this: NuGet/Home#13937

[jeffhandley]

Assigned to @adamsitnik for triage.

[adamsitnik]

I like the idea of extending the message provided by IOException since it's simply a common PITA.

My main concern is that it's not just a matter of calling one sys-call. We would need to call at least four different sys-calls:

1. Start a Restart Manager session with RmStartSession.
2. Use RmRegisterResources to register the file in question.
3. Call the RmGetList function to retrieve a list of processes using the file.
4. End the Restart Manager session

And there seems to be a limit in regard to the number of opened sessions:

https://github.com/dotnet/msbuild/blob/6cd445d84e59a36c7fbb6f50b7a5a62767a6da51/src/Utilities/LockCheck.cs#L65

Of course we could handle reaching the limit by not providing the extra information. And these sys-calls are available on all Windows that we support (Vista+, Server 2008+).

@jkotas @stephentoub do we provide any extra information similar to this for any other exceptions in .NET? Would doing that be against any guidance we have?

[jkotas]

do we provide any extra information similar to this for any other exceptions in .NET? Would doing that be against any guidance we have?

I would be worried about overhead and reliability of providing this additional information.

• We encourage exception handling to handle file I/O errors. There are likely applications out there where throwing these exceptions is common and this additional error logging would introduce prohibitive overhead.
• Restart manager is a setup technology. It is not designed and tested to be used at scale that this error logging would potentially expose it to. We would likely see unexpected fallouts from this change. (It would not be the first time that we try to use OS APIs for scenarios that they were not designed and tested for.)

[tmat]

@jkotas Would this mitigate your concerns?

allow controlling it by an AppContext switch, so that a host app could decide whether or not to include this information in all instances of IOException caused by access to a file used by another process.

[jkotas]

Somewhat. AppContext switches are process global. They do not work when different parts of the app want them configured differently. We tend to prefer APIs. AppContext switches are solution of last resort.

[tmat]

In scenarios like msbuild, it'd actually be beneficial if the host (msbuild) set this flag for all exceptions that occur within the build, so that individual tasks don't need to be updated with extra logic.