www.cendio.com
Bug 5096 - Copy/paste issue between client and server applications
: Copy/paste issue between client and server applications
Status: CLOSED FIXED
: ThinLinc
VNC
: trunk
: PC Unknown
: P2 Normal
: 4.3.0
Assigned To:
:
:
: 5111
:
  Show dependency treegraph
 
Reported: 2014-04-16 09:13 by
Modified: 2017-04-26 08:36 (History)
Acceptance Criteria:


Attachments
clipboard monitor (2.33 KB, text/x-csrc)
2014-05-22 09:29, Henrik Andersson
Details
FLTK clipboard monitor (1017 bytes, text/x-csrc)
2014-05-23 09:51, Henrik Andersson
Details


Note

You need to log in before you can comment on or make changes to this bug.


Description From cendio 2014-04-16 09:13:46
We have had several reports of copy/paste between the client and ThinLinc
session ceasing to work after some period of time. Have been unable to
reproduce this after a quick test.

Reported client platforms: Windows 7 64-bit and Windows Vista 32-bit.

Reported client-side applications: IE, Chrome, Notepad++ and Word.

Reported server-side applications: vi, gedit

Reported server-side OS: CentOS 6.5

Reported server-side DE: GNOME, KDE

Reported ThinLinc version: 4.1.1

See issue 15079.
------- Comment #1 From cendio 2014-04-16 10:24:45 -------
see also http://sourceforge.net/p/tigervnc/bug-tracker/144/
------- Comment #2 From cendio 2014-05-05 15:38:01 -------
With some investigation on this bug, I found issues with FLTK clipboard
patches, created bug 5111 for this issue.

Fixed it and added logging to vncviewer and with help from external customer we
could reproduce an issue:

Start two ThinLinc clients and logon into two sessions, close the first client
and clipboard stops working in the second. This indicates there is some problem
with the cleanup of clipboard monitoring.
------- Comment #3 From cendio 2014-05-05 15:55:51 -------
Added additional logging to fltk and identified that
fl_clipboard_notify_untarget() was never called upon close of window neither
was Fl_Window::hide() which should be called upon destruction of Fl_Window..

Added logging into vncviewer and identified that constructors were never called
and found that CleanupSignalHandler was called in main, which just does an
exit(1) without destroying CConn which holds the DesktopWindow (Fl_Window).

The main problem here is that WM_CLOSE posts a WM_QUIT (PostQuitMessage())
which will raise a SIGTERM and short-circuit the clean up of vncviewer with
just an exit(1). This needs to be fixed.

The second issue is fixed in upstream patches, a WM_CLOSE should never post a
WM_QUIT. This is a oneliner, remove PostQuitMessage() from WM_CLOSE in
FL_win32.cxx.

I have fixed WM_CLOSE and verified that this fixes the reproducable issue on
comment #2
------- Comment #4 From cendio 2014-05-22 09:29:51 -------
Created an attachment (id=544) [details]
clipboard monitor

Created a clipboard monitor program which reveals that fltk still doesn't
removes a window from the monitor chain the correct way.
------- Comment #5 From cendio 2014-05-23 09:50:07 -------
Further testing, I also created a clipboard monitor using fltk and I couldn't
reproduce the issue, however this fltk program used Fl:run() which vncviewer
doesn't. Tried to mimic vncviewer mainloop and I could recreate the issue.

Here is the mainloop that reproduce the issue;

  while (!exit_loop) {
    Fl::wait(0.1);
  }
  window->hide();

It triggers a different codepath from Fl:run() so next thing is to try trace
the Win32 API calls to identify orders of calls.
------- Comment #6 From cendio 2014-05-23 09:51:06 -------
Created an attachment (id=545) [details]
FLTK clipboard monitor

Test program for fltk clipboard monitor, with faulty main loop enabled.
------- Comment #7 From cendio 2014-05-23 14:30:31 -------
(In reply to comment #5)
> Further testing, I also created a clipboard monitor using fltk and I couldn't
> reproduce the issue, however this fltk program used Fl:run() which vncviewer
> doesn't. Tried to mimic vncviewer mainloop and I could recreate the issue.
> 
> Here is the mainloop that reproduce the issue;
> 
>   while (!exit_loop) {
>     Fl::wait(0.1);
>   }
>   window->hide();
> 
> It triggers a different codepath from Fl:run() so next thing is to try trace
> the Win32 API calls to identify orders of calls.

This is a problem which I created for myself having the wrong patches and build
system for the testing binaries.

I now have a patch that fixes 2 issues as identified before which solves the
clipboard issues.

1) WM_CLOSE does a PostQuitMessages() that reset the cleanup code.

2) CHANGECBCHAIN that correctly handles and dipatches the message
------- Comment #8 From cendio 2014-05-26 12:27:49 -------
There is also another problem, fltk hooks into clipboard monitoring even if no
handler is registered. This means that tlclient also is affected by the
clipboard issues.

Reproducible using following steps;

1. Start and login into a session using ThinLinc client.

2. Start a second ThinLinc client and close it without loggin into a session.

3. Copy'n'Paste into the session created in step 1 fails.
------- Comment #9 From cendio 2014-06-16 12:38:31 -------
(In reply to comment #8)
> There is also another problem, fltk hooks into clipboard monitoring even if no
> handler is registered. This means that tlclient also is affected by the
> clipboard issues.
> 

This issue is handled and solved on bug #5111 comment #4
------- Comment #10 From cendio 2014-10-02 13:29:26 -------
Still broken. Tested:

 1. Started client and logged in
 2. Started second client and logged in
 3. Closed second client using button on title bar
 4. Clipboard now broken to first client
------- Comment #11 From cendio 2014-10-02 14:15:24 -------
We've also overlooked the case of vncviewer getting killed by tlclient.bin
(e.g. when you remove a smart card).

Perhaps FLTK should register an atexit() handler to ensure cleanup?
------- Comment #12 From cendio 2014-10-02 14:36:32 -------
(In reply to comment #2)
> Fixed it and added logging to vncviewer and with help from external customer we
> could reproduce an issue:
> 
issue 15119
------- Comment #13 From cendio 2014-10-07 17:04:13 -------
The problem was caused here:

(In reply to comment #7)
> 
> 1) WM_CLOSE does a PostQuitMessages() that reset the cleanup code.
> 

This fix accidentally also changed the return code from WM_CLOSE. This causes
Windows to automatically destroy the window, messing up our cleanup. The window
handle is then invalid by the time we get around to unregistering for clipboard
notifications.
------- Comment #14 From cendio 2014-10-07 17:10:31 -------
(In reply to comment #13)
> The problem was caused here:
> 
> (In reply to comment #7)
> > 
> > 1) WM_CLOSE does a PostQuitMessages() that reset the cleanup code.
> > 
> 
> This fix accidentally also changed the return code from WM_CLOSE. This causes
> Windows to automatically destroy the window, messing up our cleanup. The window
> handle is then invalid by the time we get around to unregistering for clipboard
> notifications.

Fixed in r29471.
------- Comment #15 From cendio 2014-10-09 14:50:05 -------
(In reply to comment #11)
> We've also overlooked the case of vncviewer getting killed by tlclient.bin
> (e.g. when you remove a smart card).
> 
> Perhaps FLTK should register an atexit() handler to ensure cleanup?

Fixed in r29481.
------- Comment #16 From cendio 2014-10-10 14:46:49 -------
Checked nightly build and it works well.

Tester should test as follows:

 - Start three clients.
 - Verify that clipboard works in all three.
 - Close the second client.
 - Verify that clipboard works in the remaining two.
 - Close the remaining two.
 - Start three new clients.
 - Verify that clipboard works in all three.
 - Close the last client.
 - Verify that clipboard works in the remaining two.

Repeat this process, but instead of closing the client, have it close as a
result of the smart card being removed. I.e. two clients should be
authenticated using password, and one using a smart card.
------- Comment #17 From cendio 2014-10-13 12:46:08 -------
(In reply to comment #16)
> Checked nightly build and it works well.
> 
> Tester should test as follows:
> 
>  - Start three clients.
>  - Verify that clipboard works in all three.
>  - Close the second client.
>  - Verify that clipboard works in the remaining two.
>  - Close the remaining two.
>  - Start three new clients.
>  - Verify that clipboard works in all three.
>  - Close the last client.
>  - Verify that clipboard works in the remaining two.
> 
> Repeat this process, but instead of closing the client, have it close as a
> result of the smart card being removed. I.e. two clients should be
> authenticated using password, and one using a smart card.

Works fine with build 4521.
------- Comment #18 From 2017-04-26 05:33:30 -------
I am experiencing almost identical symptoms to this bug, however I am using
client version 4.7 with server version 4.1.1, and I assumed it was a problem
with the client. Must the server be upgraded too, to fix this bug? Thanks.
------- Comment #19 From cendio 2017-04-26 08:36:58 -------
(In reply to comment #18)
> I am experiencing almost identical symptoms to this bug, however I am using
> client version 4.7 with server version 4.1.1, and I assumed it was a problem
> with the client. Must the server be upgraded too, to fix this bug? Thanks.

No, the changes on this bug was only for the client. However, on bug 5772, we
changed the server implementation, so it is still possible that upgrading the
server will resolve your issues.