Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

writing to stdout from a stopexecutable will eventually cause the process to hang #218

Closed
pnikonowicz opened this issue May 10, 2017 · 3 comments · Fixed by #220
Closed

writing to stdout from a stopexecutable will eventually cause the process to hang #218

pnikonowicz opened this issue May 10, 2017 · 3 comments · Fixed by #220
Labels
bug
Milestone
2.1.1

Comments

@pnikonowicz
Copy link

This happens because the stop executable's stdout stream is redirected to the parent process but is never drained, thus eventually causing the stopexecutable's stdout writer to hang.

I recommend either setting these flags to false here:
https://github.com/kohsuke/winsw/blob/0da384180419ae5557685736e1e0e5fdab17eeff/src/Core/WinSWCore/Util/ProcessHelper.cs#L141

Or actually consuming the processes stream and piping it to a log file.

For more info on how to redirect stdout from a process, see here:
https://msdn.microsoft.com/en-us/library/system.diagnostics.processstartinfo.redirectstandardoutput(v=vs.110).aspx
@oleg-nenashev
Copy link
Member

Or actually consuming the processes stream and piping it to a log file

This one sounds like a plan

@oleg-nenashev oleg-nenashev mentioned this issue May 23, 2017
Merged
oleg-nenashev added a commit that referenced this issue Jun 5, 2017
@oleg-nenashev
Merge pull request #220 from greenhouse-org/fix_218
10c1ec3 
[Issue #218] StdOut was not being redirected properly and was causing…
@oleg-nenashev
Copy link
Member

The change also impacts the start (common) executable. According to the brief testing it is not correct in that case because the logs are being drained correctly there to logfiles. I will revisit the implementation, hopefully within 1-2 days

oleg-nenashev added a commit to oleg-nenashev/winsw that referenced this issue Jun 8, 2017
@oleg-nenashev
Issue winsw#218 - ProcessHelper#StartProcessAndCallbackForExit() shou…
0509237 
…ld redirect STDOUT/STDERR when LogHandler is defined

It restores logging of executables, which has been broken in winsw#220.
Not a regression, because the change has not been released yet
oleg-nenashev added a commit to oleg-nenashev/winsw that referenced this issue Jun 8, 2017
@oleg-nenashev
Issue winsw#218 - Also support managing Stdin, which is required in t…
69857d5 
…he main executable logic
oleg-nenashev added a commit that referenced this issue Jun 11, 2017
@oleg-nenashev
Merge pull request #224 from oleg-nenashev/bug/Issue218_fix2
a2c2a9d 
Issue #218 - ProcessHelper#StartProcessAndCallbackForExit() should redirect STDOUT/STDERR when LogHandler is defined
@oleg-nenashev
Copy link
Member

The fix has been released in 2.1.1
Once I get #213 integrated, I will setup forwarding of the output to the wrapper log

@oleg-nenashev oleg-nenashev mentioned this issue Jun 12, 2017
Open
oleg-nenashev added a commit to oleg-nenashev/windows-slave-installer-module that referenced this issue Aug 17, 2017
@oleg-nenashev
[JENKINS-46282] - Update WinSW from 2.1.0 to 2.1.2
d7454c7 
Fixes [JENKINS-46282](https://issues.jenkins-ci.org/browse/JENKINS-46282), which impacts the default installation.
Also updates Parent POM in the module

Full list of fixes:

- JENKINS-46282 - Runaway Process Killer extension was not using the stopTimeoutMs parameter
- [WinSW Issue #206](winsw/winsw#206) - Prevent printing of log entries in the `status` command
- [WinSW Issue #218](winsw/winsw#218) - Prevent hanging of the stop executable when its logs are not being drained do the parent process

Full Diff: winsw/winsw@winsw-v2.1.0...winsw-v2.1.2
@oleg-nenashev oleg-nenashev mentioned this issue Aug 17, 2017
Merged
oleg-nenashev added a commit to oleg-nenashev/jenkins that referenced this issue Aug 17, 2017
@oleg-nenashev
[JENKINS-46282] - Update WinSW from 2.1.0 to 2.1.2
a6ed239 
Fixes [JENKINS-46282](https://issues.jenkins-ci.org/browse/JENKINS-46282), which impacts the default installation.
Also updates Parent POM in the module

Full list of fixes:

- JENKINS-46282 - Runaway Process Killer extension was not using the stopTimeoutMs parameter
- [WinSW Issue jenkinsci#206](winsw/winsw#206) - Prevent printing of log entries in the `status` command
- [WinSW Issue jenkinsci#218](winsw/winsw#218) - Prevent hanging of the stop executable when its logs are not being drained do the parent process

Full Diff: winsw/winsw@winsw-v2.1.0...winsw-v2.1.2
@oleg-nenashev oleg-nenashev mentioned this issue Aug 17, 2017
Merged
4 tasks
oleg-nenashev added a commit to jenkinsci/jenkins that referenced this issue Aug 19, 2017
@oleg-nenashev
[JENKINS-46282] - Update WinSW from 2.1.0 to 2.1.2 (#2987)
100202c 
* [JENKINS-46282] - Update WinSW from 2.1.0 to 2.1.2

Fixes [JENKINS-46282](https://issues.jenkins-ci.org/browse/JENKINS-46282), which impacts the default installation.
Also updates Parent POM in the module

Full list of fixes:

- JENKINS-46282 - Runaway Process Killer extension was not using the stopTimeoutMs parameter
- [WinSW Issue #206](winsw/winsw#206) - Prevent printing of log entries in the `status` command
- [WinSW Issue #218](winsw/winsw#218) - Prevent hanging of the stop executable when its logs are not being drained do the parent process

Full Diff: winsw/winsw@winsw-v2.1.0...winsw-v2.1.2

* [JENKINS-46282] - Pick the released version of Windows Agent Installer
hplatou pushed a commit to hplatou/jenkins that referenced this issue Aug 21, 2017
@oleg-nenashev
[JENKINS-46282] - Update WinSW from 2.1.0 to 2.1.2 (jenkinsci#2987)
f19c339 
* [JENKINS-46282] - Update WinSW from 2.1.0 to 2.1.2

Fixes [JENKINS-46282](https://issues.jenkins-ci.org/browse/JENKINS-46282), which impacts the default installation.
Also updates Parent POM in the module

Full list of fixes:

- JENKINS-46282 - Runaway Process Killer extension was not using the stopTimeoutMs parameter
- [WinSW Issue jenkinsci#206](winsw/winsw#206) - Prevent printing of log entries in the `status` command
- [WinSW Issue jenkinsci#218](winsw/winsw#218) - Prevent hanging of the stop executable when its logs are not being drained do the parent process

Full Diff: winsw/winsw@winsw-v2.1.0...winsw-v2.1.2

* [JENKINS-46282] - Pick the released version of Windows Agent Installer
olivergondza pushed a commit to jenkinsci/jenkins that referenced this issue Sep 4, 2017
@oleg-nenashev @olivergondza
[JENKINS-46282] - Update WinSW from 2.1.0 to 2.1.2 (#2987)
dfd44ec 
* [JENKINS-46282] - Update WinSW from 2.1.0 to 2.1.2

Fixes [JENKINS-46282](https://issues.jenkins-ci.org/browse/JENKINS-46282), which impacts the default installation.
Also updates Parent POM in the module

Full list of fixes:

- JENKINS-46282 - Runaway Process Killer extension was not using the stopTimeoutMs parameter
- [WinSW Issue #206](winsw/winsw#206) - Prevent printing of log entries in the `status` command
- [WinSW Issue #218](winsw/winsw#218) - Prevent hanging of the stop executable when its logs are not being drained do the parent process

Full Diff: winsw/winsw@winsw-v2.1.0...winsw-v2.1.2

* [JENKINS-46282] - Pick the released version of Windows Agent Installer

(cherry picked from commit 100202c)
@nxtn nxtn changed the title writing to stdout from a stopexecutable will eventaully cause the process to hang writing to stdout from a stopexecutable will eventually cause the process to hang Apr 11, 2020
@nxtn nxtn added this to the 2.1.1 milestone Apr 13, 2020
@nxtn nxtn closed this as completed Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
2.1.1
Development

Successfully merging a pull request may close this issue.

[Issue #218] StdOut was not being redirected properly and was causing…
3 participants
@pnikonowicz @oleg-nenashev @nxtn