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.

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

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

Open
pnikonowicz opened this issue May 10, 2017 · 3 comments

Comments

2 participants
@pnikonowicz
Copy link

commented May 10, 2017

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:

ps.RedirectStandardInput = true; // this creates a pipe for stdin to the new process, instead of having it inherit our stdin.

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 oleg-nenashev added the bug label May 11, 2017

@oleg-nenashev

This comment has been minimized.

Copy link
Collaborator

commented May 11, 2017

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

This one sounds like a plan

pnikonowicz pushed a commit to greenhouse-org/winsw that referenced this issue May 17, 2017

bot
[Issue kohsuke#218]
StdOut was not being redirected properly and was causing the
child process to hang.

This fixes that problem.

pnikonowicz pushed a commit to greenhouse-org/winsw that referenced this issue May 17, 2017

pnikonowicz pushed a commit to greenhouse-org/winsw that referenced this issue May 17, 2017

pnikonowicz pushed a commit to greenhouse-org/winsw that referenced this issue May 18, 2017

pnikonowicz pushed a commit to greenhouse-org/winsw that referenced this issue May 19, 2017

davidjahn pushed a commit to greenhouse-org/winsw that referenced this issue May 30, 2017

davidjahn pushed a commit to greenhouse-org/winsw that referenced this issue May 30, 2017

oleg-nenashev added a commit that referenced this issue Jun 5, 2017

Merge pull request #220 from greenhouse-org/fix_218
[Issue #218] StdOut was not being redirected properly and was causing…
@oleg-nenashev

This comment has been minimized.

Copy link
Collaborator

commented Jun 5, 2017

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

Issue kohsuke#218 - ProcessHelper#StartProcessAndCallbackForExit() sh…
…ould redirect STDOUT/STDERR when LogHandler is defined

It restores logging of executables, which has been broken in kohsuke#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 added a commit that referenced this issue Jun 11, 2017

Merge pull request #224 from oleg-nenashev/bug/Issue218_fix2
Issue #218 - ProcessHelper#StartProcessAndCallbackForExit() should redirect STDOUT/STDERR when LogHandler is defined
@oleg-nenashev

This comment has been minimized.

Copy link
Collaborator

commented Jun 12, 2017

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 added a commit to oleg-nenashev/windows-slave-installer-module that referenced this issue Aug 17, 2017

[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](kohsuke/winsw#206) - Prevent printing of log entries in the `status` command
- [WinSW Issue #218](kohsuke/winsw#218) - Prevent hanging of the stop executable when its logs are not being drained do the parent process

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

oleg-nenashev added a commit to oleg-nenashev/jenkins that referenced this issue Aug 17, 2017

[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](kohsuke/winsw#206) - Prevent printing of log entries in the `status` command
- [WinSW Issue jenkinsci#218](kohsuke/winsw#218) - Prevent hanging of the stop executable when its logs are not being drained do the parent process

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

oleg-nenashev added a commit to jenkinsci/jenkins that referenced this issue Aug 19, 2017

[JENKINS-46282] - Update WinSW from 2.1.0 to 2.1.2 (#2987)
* [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](kohsuke/winsw#206) - Prevent printing of log entries in the `status` command
- [WinSW Issue #218](kohsuke/winsw#218) - Prevent hanging of the stop executable when its logs are not being drained do the parent process

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

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

hplatou added a commit to hplatou/jenkins that referenced this issue Aug 21, 2017

[JENKINS-46282] - Update WinSW from 2.1.0 to 2.1.2 (jenkinsci#2987)
* [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](kohsuke/winsw#206) - Prevent printing of log entries in the `status` command
- [WinSW Issue jenkinsci#218](kohsuke/winsw#218) - Prevent hanging of the stop executable when its logs are not being drained do the parent process

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

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

olivergondza added a commit to jenkinsci/jenkins that referenced this issue Sep 4, 2017

[JENKINS-46282] - Update WinSW from 2.1.0 to 2.1.2 (#2987)
* [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](kohsuke/winsw#206) - Prevent printing of log entries in the `status` command
- [WinSW Issue #218](kohsuke/winsw#218) - Prevent hanging of the stop executable when its logs are not being drained do the parent process

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

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

(cherry picked from commit 100202c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.