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

Feature/make play stop wait for java process to finish #1239

Conversation

LouisJackman
Copy link
Contributor

play stop finishes before the Java process forked from the Python script has finished running. It will now wait until the Java process finishes before finishing itself.

This strikes me as the least surprising behaviour. We have a systemd service that restarts our application by running play stop and play start. However, this means the two can end up running side-by-side for a while, which can obviously lead to various problems.

This PR also fixes the issue of Ctrl-C not stopping play run properly. SIGINT was previously only trapped after the main process was interrupted, which isn't useful as far as I can see. Ctrl-C now prints "Terminating Java" and waits until Java has finished.

The changes were tested with Python 2.7 on Ubuntu Linux 18.04. If an earlier Python must be used and tested against, let me know and I'll update the PR. Thanks.

@LouisJackman
Copy link
Contributor Author

LouisJackman commented Jun 8, 2018

The error is due to failing tests. All failing tests are reporting the same error, but it seems very unlikely to be related to the code changes in the PR.

Can someone confirm whether this is a CI issue and not related to the PR's code?

java.lang.RuntimeException: java.util.concurrent.ExecutionException: java.net.ConnectException: connection timed out: google.com/74.125.69.113:80
	at play.libs.ws.WSAsync$WSAsyncRequest.post(WSAsync.java:308)
	at WSTest.multiplePostContentTest2(WSTest.java:45)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at play.test.PlayJUnitRunner$StartPlay$2$1.evaluate(PlayJUnitRunner.java:128)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at play.test.PlayJUnitRunner.run(PlayJUnitRunner.java:70)
	at org.junit.runners.Suite.runChild(Suite.java:128)
	at org.junit.runners.Suite.runChild(Suite.java:27)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:115)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:105)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:94)
	at play.test.TestEngine.run(TestEngine.java:188)
	at controllers.TestRunner$1.doJobWithResult(TestRunner.java:101)
	at controllers.TestRunner$1.doJobWithResult(TestRunner.java:1)
	at play.jobs.Job$2.apply(Job.java:224)
	at play.db.jpa.JPA.withTransaction(JPA.java:285)
	at play.db.jpa.JPA.withinFilter(JPA.java:238)
	at play.db.jpa.JPAPlugin$TransactionalFilter.withinFilter(JPAPlugin.java:304)
	at play.jobs.Job.withinFilter(Job.java:201)
	at play.jobs.Job.call(Job.java:220)
	at play.jobs.Job$1.call(Job.java:135)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.util.concurrent.ExecutionException: java.net.ConnectException: connection timed out: google.com/74.125.69.113:80
	at com.ning.http.client.providers.netty.future.NettyResponseFuture.abort(NettyResponseFuture.java:231)
	at com.ning.http.client.providers.netty.request.NettyConnectListener.onFutureFailure(NettyConnectListener.java:137)
	at com.ning.http.client.providers.netty.request.NettyConnectListener.operationComplete(NettyConnectListener.java:145)
	at org.jboss.netty.channel.DefaultChannelFuture.notifyListener(DefaultChannelFuture.java:409)
	at org.jboss.netty.channel.DefaultChannelFuture.notifyListeners(DefaultChannelFuture.java:400)
	at org.jboss.netty.channel.DefaultChannelFuture.setFailure(DefaultChannelFuture.java:362)
	at org.jboss.netty.channel.socket.nio.NioClientBoss.processConnectTimeout(NioClientBoss.java:142)
	at org.jboss.netty.channel.socket.nio.NioClientBoss.process(NioClientBoss.java:83)
	at org.jboss.netty.channel.socket.nio.AbstractNioSelector.run(AbstractNioSelector.java:337)
	at org.jboss.netty.channel.socket.nio.NioClientBoss.run(NioClientBoss.java:42)
	at org.jboss.netty.util.ThreadRenamingRunnable.run(ThreadRenamingRunnable.java:108)
	at org.jboss.netty.util.internal.DeadLockProofWorker$1.run(DeadLockProofWorker.java:42)
	... 3 more
Caused by: java.net.ConnectException: connection timed out: google.com/74.125.69.113:80
	at com.ning.http.client.providers.netty.request.NettyConnectListener.onFutureFailure(NettyConnectListener.java:133)
	... 13 more
Caused by: org.jboss.netty.channel.ConnectTimeoutException: connection timed out: google.com/74.125.69.113:80
	at org.jboss.netty.channel.socket.nio.NioClientBoss.processConnectTimeout(NioClientBoss.java:139)
	... 8 more

@LouisJackman
Copy link
Contributor Author

The failed tests in question, WSTest, passe on my local machine. This seems like a CI issue. The results specifically are:

  • getWithVitualhostTest: OK
  • multiplePostContentTest2: FAILED
  • multipleGetStreamKo: OK
  • multipleGetStreamOk: FAILED

@asolntsev
Copy link
Contributor

asolntsev commented Jun 25, 2018

@LouisJackman I have restarted the build. Yes, probably it's just a flaky tests. Let's see.

UPD. You see, it was a flaky tests. :)

@asolntsev asolntsev self-requested a review June 25, 2018 22:02
Copy link
Contributor

@asolntsev asolntsev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xael-fry Seems to be OK. Can be merged.

@asolntsev asolntsev merged commit c91f201 into playframework:master Jul 30, 2018
@asolntsev
Copy link
Contributor

@xael-fry Well, I have merged this PR.

@LouisJackman Thank you!

@xael-fry xael-fry added this to the 1.5.2 milestone Aug 22, 2018
xael-fry pushed a commit to xael-fry/play that referenced this pull request Aug 23, 2018
* Only pay for message construction on 'real' errors
* Wait for process termination before leaving Play process
xael-fry added a commit that referenced this pull request Aug 23, 2018
…ava-process-to-finish

[#1239] make play stop wait for java process to finish
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants