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

[JENKINS-50166] Option to not to kill process on interruption #3351

Closed
wants to merge 1 commit into from

Conversation

@apetres
Copy link
Contributor

apetres commented Mar 14, 2018

Processes created by Launcher are always killed on interruption. This is undesired if the caller (plugin) wants to do some action before the process is killed. My solution is to add a flag to Launcher which prevents the killing of the created process on (build) interruption.

This is a follow-up for #3293.

See JENKINS-50166.

Proposed changelog entry

  • Developer: Add support to prevent killing processes created by Launcher on interruption

Desired reviewers

@oleg-nenashev
@daniel-beck

@apetres apetres force-pushed the Itiviti:JENKINS-50166 branch from c743470 to a010006 Mar 14, 2018
@oleg-nenashev
Copy link
Member

oleg-nenashev commented Mar 17, 2018

Generally needs feedback from @daniel-beck, I'd guess

@oleg-nenashev
Copy link
Member

oleg-nenashev commented Mar 31, 2018

@daniel-beck gentle ping

@apetres
Copy link
Contributor Author

apetres commented Apr 18, 2018

@daniel-beck could you review this PR?

@daniel-beck
Copy link
Member

daniel-beck commented Apr 21, 2018

Still lagging behind my review queue, sorry about that.

@oleg-nenashev
Copy link
Member

oleg-nenashev commented May 5, 2018

@daniel-beck gentle ping

}

/**
* @param err
* null to redirect stderr to stdout.
*/
public LocalProc(String[] cmd,String[] env,InputStream in,OutputStream out,OutputStream err,File workDir) throws IOException {
public LocalProc(String[] cmd,String[] env,InputStream in,OutputStream out,OutputStream err,File workDir, boolean dontKillWhenInterrupted) throws IOException {

This comment has been minimized.

Copy link
@daniel-beck

daniel-beck May 14, 2018

Member

Looks like a breaking change.

* Indicates that the process should not be killed on interruption.
* @since TODO
*/
protected boolean dontKillWhenInterrupted;

This comment has been minimized.

Copy link
@daniel-beck

daniel-beck May 14, 2018

Member

doNotKill… perhaps?

*
* <p>
* Note that the process can (and should) be killed
* via {@link Proc#kill()} when custom action is done.

This comment has been minimized.

Copy link
@daniel-beck

daniel-beck May 14, 2018

Member

This indicates to me that the better API design is to submit a callback to be invoked between interrupting and killing, rather than just a flag to skip it.

This comment has been minimized.

Copy link
@apetres

apetres Jul 16, 2018

Author Contributor

I agree. But it makes the implementation more complicated...
@oleg-nenashev could you recommend a way to pass the callback? I tried to pass hudson.remoting.Callable but I got NotSerializableExceptions.

@daniel-beck daniel-beck removed their assignment May 15, 2018
@varyvol
Copy link
Contributor

varyvol commented Mar 28, 2019

@apetres do you have plans to keep working on this?

@apetres
Copy link
Contributor Author

apetres commented Mar 28, 2019

Not really. Seems too complicated to implement it the proper way (submit callback over remoting) and I don't have time right to do it.
I will stick with a custom Jenkins build for now.

@apetres apetres closed this Mar 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.