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

Support Build Cancellation in `JavaExec` Task #1128

Closed
lacasseio opened this Issue Jan 9, 2017 · 12 comments

Comments

Projects
None yet
@lacasseio
Member

lacasseio commented Jan 9, 2017

Edit: Improved description to reference the more generic solution (build cancel event) as oppose to the specific usage of this feature (Ctrl-D) explained by Gary's comment.

Expected Behavior

When JavaExec task waits for the completion of the Java application, it should support cancel event and terminate the Java application only. This exposes a deterministic build cancellation to Java shutdown hook execution delay to the user as well as avoid killing a perfectly healthy daemon. It would also be possible to support continuous mode for any JavaExec task.

Current Behavior

JavaExec doesn't support cancel event. The only way to cancel an application ran through that task is to send a Ctrl-C which will cause the daemon to exit as the JavaExec task doesn't return:

  1. There is a, up to, 10 seconds delay between Ctrl-C and the execution of the shutdown hook in the Java application
  2. The System.{out|err} output of the Java application isn't piped to the console.

Context

Initial discussion in #1109

Steps to Reproduce (for bugs)

build.gradle:

apply plugin: 'java'
apply plugin: 'application'

mainClassName = 'Runner'

src/main/java/Runner.java

import java.io.File;
import java.util.Random;
import java.util.concurrent.CompletableFuture;

public class Runner {
    public static void main(String[] args) throws Exception {
        Runtime.getRuntime().addShutdownHook(new Thread(() -> {
            System.out.println("Clean shutdown");
        }));
        File lock = new File("LOCK" + new Random().nextInt(20));
        lock.createNewFile();
        lock.deleteOnExit();
        System.out.println("Created lock");
        new CompletableFuture<Void>().get();
    }
}
@pkubowicz

This comment has been minimized.

Show comment
Hide comment
@pkubowicz

pkubowicz Jan 9, 2017

Contributor

One more comment: the 10s delay is not acceptable in many scenarios. For example when developing a server application you will frequently restart you application. You will require a fast restart and a clean restart, that leaves a coherent state.

Idea 2016.3 has an option to delegate run task to Gradle. Then, when you run the application, Idea executes 'run' for you, although you may not know what exactly is happening under the hood. When not using delegating, using the 'restart' button for a server app in Idea is smooth and fast. With delegating to Gradle, it often failed for me because of port still being used. Probably this is because Gradle daemon waits 10s but Idea starts a new build immediately. Also restarting feels not right, because you don't see console logs about server being stopped - again, because of the 10s delay of running shutdown hooks.

Contributor

pkubowicz commented Jan 9, 2017

One more comment: the 10s delay is not acceptable in many scenarios. For example when developing a server application you will frequently restart you application. You will require a fast restart and a clean restart, that leaves a coherent state.

Idea 2016.3 has an option to delegate run task to Gradle. Then, when you run the application, Idea executes 'run' for you, although you may not know what exactly is happening under the hood. When not using delegating, using the 'restart' button for a server app in Idea is smooth and fast. With delegating to Gradle, it often failed for me because of port still being used. Probably this is because Gradle daemon waits 10s but Idea starts a new build immediately. Also restarting feels not right, because you don't see console logs about server being stopped - again, because of the 10s delay of running shutdown hooks.

@ghale

This comment has been minimized.

Show comment
Hide comment
@ghale

ghale Jan 9, 2017

Member

The 10s delay is a byproduct of the fact that the task never finishes. What happens when you send a ctrl-c is that it terminates the daemon client. The client disappearing then causes the daemon to cancel any running builds associated with that client. The build cancellation logic (as it currently exists) checks in between tasks to see if the build has been canceled, so it relies on the currently running task(s) to complete before it can stop the build. In this case, the JavaExec task never completes, so the 10s delay is the timeout on the daemon waiting for the currently JavaExec task to finish. When this timeout expires, the daemon has no guarantee that the task will ever finish (in this case it won't) so it kills itself to clean up the resources.

So, another way to handle this is to provide better mechanics for tasks to respond to cancellation. For instance, allow tasks to register a cancellation hook so that when a cancellation occurs, we can notify a running task that we are waiting for it to finish and it could stop early rather than running to completion. In the case of the JavaExec task, it could stop the child process and finish. In the case of say a Test task, it could stop executing tests. Etc.

For a more controlled shutdown (i.e. allowing any stdout/stderr to make it back to the client) we might generalize ctrl-d to more formally mean "cancel the build" in which case it would send a cancellation request to the daemon which would work exactly the same as the ctrl-c case except that the client would remain connected until the cancellation was complete (and any output would be piped back to the console).

Member

ghale commented Jan 9, 2017

The 10s delay is a byproduct of the fact that the task never finishes. What happens when you send a ctrl-c is that it terminates the daemon client. The client disappearing then causes the daemon to cancel any running builds associated with that client. The build cancellation logic (as it currently exists) checks in between tasks to see if the build has been canceled, so it relies on the currently running task(s) to complete before it can stop the build. In this case, the JavaExec task never completes, so the 10s delay is the timeout on the daemon waiting for the currently JavaExec task to finish. When this timeout expires, the daemon has no guarantee that the task will ever finish (in this case it won't) so it kills itself to clean up the resources.

So, another way to handle this is to provide better mechanics for tasks to respond to cancellation. For instance, allow tasks to register a cancellation hook so that when a cancellation occurs, we can notify a running task that we are waiting for it to finish and it could stop early rather than running to completion. In the case of the JavaExec task, it could stop the child process and finish. In the case of say a Test task, it could stop executing tests. Etc.

For a more controlled shutdown (i.e. allowing any stdout/stderr to make it back to the client) we might generalize ctrl-d to more formally mean "cancel the build" in which case it would send a cancellation request to the daemon which would work exactly the same as the ctrl-c case except that the client would remain connected until the cancellation was complete (and any output would be piped back to the console).

@lacasseio

This comment has been minimized.

Show comment
Hide comment
@lacasseio

lacasseio Jan 9, 2017

Member

Thanks @ghale for the detailed information. I saw in PlayRun that it handle Ctrl-D manually and I was thinking something similar to that in JavaExec. However, what you expose is probably better as the solution. I will edit the title and description to reflect your suggestion.

Member

lacasseio commented Jan 9, 2017

Thanks @ghale for the detailed information. I saw in PlayRun that it handle Ctrl-D manually and I was thinking something similar to that in JavaExec. However, what you expose is probably better as the solution. I will edit the title and description to reflect your suggestion.

@lacasseio lacasseio changed the title from Support Ctrl-D behavior in `JavaExec` task to Support Build Cancellation in `JavaExec` Task Jan 9, 2017

@big-guy

This comment has been minimized.

Show comment
Hide comment
@big-guy

big-guy Jan 9, 2017

Member

@lacasseio PlayRun handles ctrl+d manually because it predates continuous build, so run blocking the build was the way it worked.

I think we wouldn't make JavaExec work with continuous build, or at the very least, that's a separate issue. It would be better to make it easier/possible for plugins to contribute their own DeploymentHandle implementations.

Member

big-guy commented Jan 9, 2017

@lacasseio PlayRun handles ctrl+d manually because it predates continuous build, so run blocking the build was the way it worked.

I think we wouldn't make JavaExec work with continuous build, or at the very least, that's a separate issue. It would be better to make it easier/possible for plugins to contribute their own DeploymentHandle implementations.

@lacasseio

This comment has been minimized.

Show comment
Hide comment
@lacasseio

lacasseio Jan 10, 2017

Member

Thanks for pitching in @big-guy. It's very valuable feedback.

Member

lacasseio commented Jan 10, 2017

Thanks for pitching in @big-guy. It's very valuable feedback.

@pkubowicz

This comment has been minimized.

Show comment
Hide comment
@pkubowicz

pkubowicz Apr 4, 2017

Contributor

Yet another problem manifesting in Idea 2016.3 and 2017.1. If you create a Run Configuration for 'gradle run', run it, then try to close the current project or the whole Idea, you will be asked if you want to Terminate, Disconnect or Cancel. 'Terminate' will hang forever.

I reported this as https://youtrack.jetbrains.com/issue/IDEA-170956

Contributor

pkubowicz commented Apr 4, 2017

Yet another problem manifesting in Idea 2016.3 and 2017.1. If you create a Run Configuration for 'gradle run', run it, then try to close the current project or the whole Idea, you will be asked if you want to Terminate, Disconnect or Cancel. 'Terminate' will hang forever.

I reported this as https://youtrack.jetbrains.com/issue/IDEA-170956

@rroels

This comment has been minimized.

Show comment
Hide comment
@rroels

rroels Aug 1, 2017

I have similar issues where I have a JavaExec-based "run" task for testing. I'm working on an application that runs continuously in the background and it doesn't stop by itself. This means I have to ctrl-c the process which kills the Gradle daemon as discussed earlier. The shutdownhooks are not called in my app either. I have similar issues using IntelliJ when I re-run my project from the IDE with a Gradle run/build configuration that only allows one instance to be running at the same time.

Except for the annoying time delay caused by starting a new Gradle daemon every time I am also running into issues because my app cannot shut down gracefully (e.g. the WebSocket server not shutting down gracefully which keeps ports blocked for another 10-20 seconds after the ctrl-c). All of this together makes it very tedious to use Gradle for this scenario. I just want to kill my app, let it shut down gracefully and start it again which really shouldn't take more than a few seconds.

I would really like to have a method for telling the daemon to stop the running JavaExec task and to let it shut down gracefully. Preferably in a way that integrates well with the IDE. I don't consider manually executing a command to find my app pid and kill it a clean solution.

rroels commented Aug 1, 2017

I have similar issues where I have a JavaExec-based "run" task for testing. I'm working on an application that runs continuously in the background and it doesn't stop by itself. This means I have to ctrl-c the process which kills the Gradle daemon as discussed earlier. The shutdownhooks are not called in my app either. I have similar issues using IntelliJ when I re-run my project from the IDE with a Gradle run/build configuration that only allows one instance to be running at the same time.

Except for the annoying time delay caused by starting a new Gradle daemon every time I am also running into issues because my app cannot shut down gracefully (e.g. the WebSocket server not shutting down gracefully which keeps ports blocked for another 10-20 seconds after the ctrl-c). All of this together makes it very tedious to use Gradle for this scenario. I just want to kill my app, let it shut down gracefully and start it again which really shouldn't take more than a few seconds.

I would really like to have a method for telling the daemon to stop the running JavaExec task and to let it shut down gracefully. Preferably in a way that integrates well with the IDE. I don't consider manually executing a command to find my app pid and kill it a clean solution.

@Apertum

This comment has been minimized.

Show comment
Hide comment
@Apertum

Apertum Sep 17, 2017

I have similar issues. A red square button in Idea or NetBeans for stopping application is not make stop of app immediately. I Have to wait about 10 sec. Inconvenient. I haven't solved it yourself.

Apertum commented Sep 17, 2017

I have similar issues. A red square button in Idea or NetBeans for stopping application is not make stop of app immediately. I Have to wait about 10 sec. Inconvenient. I haven't solved it yourself.

@fab1an

This comment has been minimized.

Show comment
Hide comment
@fab1an

fab1an Sep 17, 2017

fab1an commented Sep 17, 2017

@orange-buffalo

This comment has been minimized.

Show comment
Hide comment
@orange-buffalo

orange-buffalo Nov 7, 2017

This is also a problem for anyone who is using Project#exec(...). Underlying implementation DefaultExecHandle uses ShutdownHookActionRegister which in turn registers a shutdown hook to cancel the created process. One example where this issue appears are npm tools to run dev servers / watch files (see srs/gradle-node-plugin#238, srs/gradle-node-plugin#143).

orange-buffalo commented Nov 7, 2017

This is also a problem for anyone who is using Project#exec(...). Underlying implementation DefaultExecHandle uses ShutdownHookActionRegister which in turn registers a shutdown hook to cancel the created process. One example where this issue appears are npm tools to run dev servers / watch files (see srs/gradle-node-plugin#238, srs/gradle-node-plugin#143).

@ahmedmohiduet

This comment has been minimized.

Show comment
Hide comment
@ahmedmohiduet

ahmedmohiduet Jan 31, 2018

I'm facing similar issue. On my Git Bash "gradle clean bootrun" now not responding for Ctrl+C

ahmedmohiduet commented Jan 31, 2018

I'm facing similar issue. On my Git Bash "gradle clean bootrun" now not responding for Ctrl+C

@oehme oehme added this to the 4.8 RC1 milestone Jun 8, 2018

@oehme

This comment has been minimized.

Show comment
Hide comment
@oehme

oehme Jun 8, 2018

Member

This was fixed in 4.8, see the duplicate ticket #2128

Member

oehme commented Jun 8, 2018

This was fixed in 4.8, see the duplicate ticket #2128

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment