Add a shorter timeout for retrieving clipboard data#81339
Add a shorter timeout for retrieving clipboard data#81339CyrusNajmabadi merged 2 commits intodotnet:mainfrom
Conversation
| do | ||
| { | ||
| hr = OleGetClipboard(ref dataObject); | ||
| if (!ErrorHandler.Succeeded(hr)) |
There was a problem hiding this comment.
Why Succeeded check here but loop terminates on S_OK check?
There was a problem hiding this comment.
There was a problem hiding this comment.
Doesn't ErrorHandler.Succeeded check that it's >= 0, not == 0? The concern I have is if OleGetClipboard returns S_FALSE, then this code could loop forever.
There was a problem hiding this comment.
Good catch. That was unintended. HAve switched this to a straight 0 check. thanks!
| if (!ErrorHandler.Succeeded(hr)) | ||
| { | ||
| if (retry == 0) | ||
| return null; |
There was a problem hiding this comment.
In Clipboard this threw. How will the behavior change affect the user experience?
There was a problem hiding this comment.
We caught the throw and returned null. So this has the same effect. It means we don't support rich copy/paste semantics. But that's better than timing out for 1 second :D
There was a problem hiding this comment.
Ah, I should have looked further down.
| if (!ErrorHandler.Succeeded(hr)) | ||
| { | ||
| if (retry == 0) | ||
| return null; |
There was a problem hiding this comment.
Ah, I should have looked further down.
| /// <summary> | ||
| /// Copied from https://github.com/dotnet/winforms/blob/0f76e65878b1a0958175f17c4360b8198f8b36ba/src/System.Windows.Forms/src/System/Windows/Forms/Clipboard.cs#L139 | ||
| /// </summary> | ||
| private static IDataObject? GetDataObject(int retryTimes, int retryDelay) |
There was a problem hiding this comment.
We already have https://github.com/dotnet/roslyn/blob/b9f26f0e3e3b81c8a0ff10449c36f6e579779b82/src/VisualStudio/Core/Def/Utilities/ClipboardHelpers.cs which you might be able to unify this code with.
| private static string GetFormat(string key) | ||
| => $"{RoslynFormat}-{key}"; | ||
|
|
||
| public bool TrySetClipboardData(string key, string data) |
There was a problem hiding this comment.
This PR is fine, but if I recall at one point we also had clipboard issues with the stack trace explorer that might have been similar. I wonder what this code would look like if we moved the clipboard manipulation to a different thread (or move it to the OOP process) entirely. I'm guessing there's some COM/STA thread semantics so it's not a trivial change, but just something to think about if we still see issues.
5 participants
Winforms exposes this (and we make use of it) already for setting clipboard data. This just exposes the same for reading clipboard data.
Noticed in local development where cutting code could take up to 1 second due to other apps on my system locking the clipboard.