www.cendio.com
Bug 4886 - clean up subprocess pipe handling on Windows
: clean up subprocess pipe handling on Windows
Status: CLOSED FIXED
: ThinLinc
Client
: trunk
: PC Unknown
: P2 Normal
: 4.2.0
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2013-11-05 14:05 by
Modified: 2014-04-14 10:36 (History)
Acceptance Criteria:


Attachments
backtrace (13.21 KB, text/plain)
2014-04-10 12:31, Henrik Andersson
Details
logfile (6.58 KB, text/plain)
2014-04-10 12:43, Henrik Andersson
Details


Note

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


Description From cendio 2013-11-05 14:05:28
We are currently a bit sloppy when it comes to handling the pipe handles for
subprocesses on Windows. AppVerifier even makes the client crash because it has
tests that make sure you use file handles in a safe way.

The primary problem is that WinPopenProcess closes the pipes, even though
ServiceProcess is still using them.

It might be sufficient to throw a couple of Fl::check() in
WinProcess::KillProcess() to make sure ServiceProcess notices that the pipe is
closed.

Related, we also need to fix WinPopenProcess so that it doesn't close the pipes
until _after_ the process has been terminated.
------- Comment #1 From cendio 2013-11-05 14:06:36 -------
(see bug 4815 and bug 4569 for the AppVerifier crashes)
------- Comment #2 From cendio 2013-12-19 13:33:52 -------
All done. Tester should run tlclient -d5 and look for PeekNamedPipe errors, and
that logging seems complete. Probably also best to test with AppVerifier for
the previously seen crashes.
------- Comment #3 From cendio 2014-04-10 12:31:39 -------
Created an attachment (id=527) [details]
backtrace

Got a new backtrace in tlclient, right after hitting connect button.
------- Comment #4 From cendio 2014-04-10 12:43:27 -------
Created an attachment (id=529) [details]
logfile
------- Comment #5 From cendio 2014-04-10 13:59:36 -------
Whatever this is, it is not this bug. AppVerifier makes PeekNamedPipe() return
ERROR_NOACCESS randomly on perfectly valid file handles. Adding bug 5087
instead.
------- Comment #6 From cendio 2014-04-14 10:36:51 -------
Tested using client build 4323, works as expected. No errors in log file when
trying to provoke killing sub processes to client.