Re: [PATCH] Cygwin: clipboard: Add workaround for ERROR_CLIPBOARD_NOT_OPEN

Hi Mark,

Thanks for reviewing!

On Fri, 12 Jun 2026 16:08:24 -0700
Mark Geisert <[email protected]> wrote:

> Hi Takashi,
> 
> On 6/8/2026 5:20 PM, Takashi Yano wrote:
> > SetClipboard/Data() and GetClipboardData() occasionally fail with
> > ERROR_CLIPBOARD_NOT_OPEN, even though OpenClipboard() succeeded if
> > NULL HWND is used. Retry until GetClipboardData() does not return
> > ERROR_CLIPBOARD_NOT_OPEN.
> > 
> > Addresses: https://cygwin.com/pipermail/cygwin/2026-February/259438.html
> > Signed-off-by: Takashi Yano <[email protected]>
> > Reviewed-by: Mark Geisert <[email protected]>
> 
> Sorry, I didn't read ^^^ as a request to review, and then forgot to ask 
> about it...

I didn't mean that. I think this was the first review.
https://cygwin.com/pipermail/cygwin/2026-February/259449.html

> 
> > ---
> >   winsup/cygwin/fhandler/clipboard.cc | 14 ++++++++++++--
> >   1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/winsup/cygwin/fhandler/clipboard.cc b/winsup/cygwin/fhandler/clipboard.cc
> > index 12691c7c1..db33d839f 100644
> > --- a/winsup/cygwin/fhandler/clipboard.cc
> > +++ b/winsup/cygwin/fhandler/clipboard.cc
> > @@ -25,11 +25,21 @@ details. */
> >   static inline bool
> >   open_clipboard ()
> >   {
> > -  const int max_retry = 10;
> > +  const int max_retry = 20;
> >     for (int i = 0; i < max_retry; i++)
> >       {
> > +      /* No appropriate HWND exists here. */
> >         if (OpenClipboard (NULL))
> > -	return true;
> > +	{
> > +	  /* SetClipboard/Data() and GetClipboardData() occasionally
> > +	     fail with ERROR_CLIPBOARD_NOT_OPEN, even though
> > +	     OpenClipboard() succeeded if NULL HWND is used.
> > +	     Retry until GetClipboardData() does not return
> > +	     ERROR_CLIPBOARD_NOT_OPEN. */
> > +	  if (GetClipboardData (CF_UNICODETEXT)
> > +	      || GetLastError () != ERROR_CLIPBOARD_NOT_OPEN)
> > +	    return true;
> 
> I don't think this 'if' is quite right.  If GetClipboardData(...) 
> succeeds, return true.  Otherwise, if GetLastError() returns 
> ERROR_CLIPBOARD_NOT_OPEN, continue the loop.  Otherwise either break the 
> loop or return false right there.

Do you mean:
    if (GetClipboardData (CF_UNICODETEXT))
      return true;
    if (GetLastError () == ERROR_CLIPBOARD_NOT_OPEN)
      continue;
    return false;
?

> Do you agree with my reasoning?  I'm open to corrections.

What happnes if the clipboard does not have TEXT, e.g. CF_BITMAP?
I think GetClipboardData (CF_UNICODETEXT) fails with ERROR_NOT_FOUND.
Even in this case, we can call SetClipboardData() successfully.
So we shoud return true in this case.

BTW, I forgot to call CloseClipboard(). Check for ERROR_NOT_FOUND
also will be added. Please review v2 patch.

-- 
Takashi Yano <[email protected]>