Misuse of SHEmptyRecycleBin in Clear-RecycleBin cmdlet #6743
Description
Overview
Clear-RecycleBin produces spurious error records when used in Windows PowerShell. (Yes I know this repo is dedicated to PowerShell Core, but since it contains the code, and UserVoice isn't quite effective, I'll just put the analysis here.)
In a freshly launched PowerShell instance, running Clear-RecycleBin -Force will produce the following error (whether or not the recycle bin is empty does not matter at all):
Clear-RecycleBin : The system cannot find the path specified
At line:1 char:1
+ Clear-RecycleBin -Force
+ ~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : InvalidOperation: (RecycleBin:String) [Clear-RecycleBin], Win32Exception
+ FullyQualifiedErrorId : FailedToClearRecycleBin,Microsoft.PowerShell.Commands.ClearRecycleBinCommand
However, subsequent invocations to the same command (with -Force) will not produce such error. Moreover, in a newly launched PowerShell instance, running Clear-RecycleBin (without -Force) and manually confirming the operation) will not write error.
Root Cause
Upon close investigation, the root cause starts on line 226 of ClearRecycleBinCommand.cs. Relevant excerpt:
uint result = NativeMethod.SHEmptyRecycleBin(IntPtr.Zero, drivePath,
NativeMethod.RecycleFlags.SHERB_NOCONFIRMATION |
NativeMethod.RecycleFlags.SHERB_NOPROGRESSUI |
NativeMethod.RecycleFlags.SHERB_NOSOUND);
int lastError = Marshal.GetLastWin32Error();
progress.PercentComplete = 100;
progress.RecordType = ProgressRecordType.Completed;
WriteProgress(progress);
// 0 is for a successful operation
// 203 comes up when trying to empty an already emptied recyclebin
// 18 comes up when there are no more files in the given recyclebin
if (!(lastError == 0 || lastError == 203 || lastError == 18))
{
Win32Exception exception = new Win32Exception(lastError);
WriteError(new ErrorRecord(exception, "FailedToClearRecycleBin", ErrorCategory.InvalidOperation, "RecycleBin"));
}The mistake here is to use GetLastWin32Error -- according to SHEmptyRecycleBin function (Windows) on MSDN, the function uses HRESULT to indicate error status. The documentation does not mention GetLastError, and it is true that the function does not call SetLastError on the caller thread before returning.
The fact that GetLastError returns 3 on the first invocation of Clear-RecycleBin -Force is purely accidental.
Playing around with SHEmptyRecycleBin indicates that it returns S_OK (0) for success (also documented), and E_UNEXPECTED (0x8000FFFF, catastrophic/unexpected failure) when the recycle bin is already empty. At this time, I would suggest remove error detection in the cmdlet, since SHEmptyRecycleBin doesn't know how to handle the case when the recycle bin is already empty at all -- we can't expect it to do any reasonable failure indication.
There could have been a time when SHEmptyRecycleBin uses SetLastError to indicate failure, and the code could have been written when the documentation wasn't updated.
I wrote a blog entry on this issue, and I found that the code is wrong in every way. If you're interested, read https://geelaw.blog/entries/clear-recyclebin-shemptyrecyclebin/
Mistakes made summarised into a list:
- not declaring
[DllImport(SetLastError = true)]and usingMarshal.GetLastWin32Errorto retrieve the possible error code set bySHEmptyRecycleBin, which won't work and will get the leftover of previous marshalled calls. - documentation for
SHEmptyRecycleBindoesn't indicate that it will callSetLastError, and recommends that one use the return value to determine error status. SHEmptyRecycleBindoesn't expect to deal with already emptied recycle bin.
I would recommend anyone using this cmdlet use Clear-RecycleBin -ErrorAction SilentlyContinue to prevent irrelevant ErrorRecord.