Skip to content

Fixed #375 "Global.onStop() is not called"#249

Merged
pk11 merged 1 commit into
playframework:masterfrom
Seyun:master
May 2, 2012
Merged

Fixed #375 "Global.onStop() is not called"#249
pk11 merged 1 commit into
playframework:masterfrom
Seyun:master

Conversation

@Seyun

@Seyun Seyun commented Apr 20, 2012

Copy link
Copy Markdown
Contributor

Fixed #375 ticket
https://play.lighthouseapp.com/projects/82401-play-20/tickets/375-globalonstop-is-not-invoked

If the application is started by 'play start' command, Global.onStop() is not called.
So I attached a hooker.

Please review it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not the following?

val server = new NettyServer(…)
Runtime.getRuntime.addShutdownHook(new Thread {
  override def run() {
    server.stop()
  }
})
Some(server)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Your suggestion looks better, so I just changed the code.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool :)
You just forgot the parenthesis after run, but that’s not so important…
Can you squash your commits into a single one?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the help.

The 2 commits were squashed.

When it comes to the parenthesis, I just followed the style just few lines above, at line 165, in the same file. :)

I have been using play frameworks for a year and love it. And I am happy to contribute to them :)

@ghost ghost assigned pk11 Apr 30, 2012
@pk11

pk11 commented Apr 30, 2012

Copy link
Copy Markdown

Can you please sign our CLA? http://www.typesafe.com/contribute/cla Thanks

@Seyun

Seyun commented May 1, 2012

Copy link
Copy Markdown
Contributor Author

I just did sign the CLA.

Thanks.

pk11 pushed a commit that referenced this pull request May 2, 2012
Fixed #375 "Global.onStop() is not called"
@pk11 pk11 merged commit 4bdb227 into playframework:master May 2, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants