brooklyncentral / brooklyn

This project has moved and is now part of the ASF
https://github.com/apache/incubator-brooklyn
72 stars 27 forks source link

Feature: register shutdown hook for management context #1406

Closed aledsage closed 10 years ago

buildhive commented 10 years ago

Brooklyn Central » brooklyn #2304 SUCCESS This pull request looks good (what's this?)

buildhive commented 10 years ago

Brooklyn Central » brooklyn #2308 SUCCESS This pull request looks good (what's this?)

grkvlt commented 10 years ago

@aledsage A few comments, although we can probably defer the idea of making Entity implement Closeable just now

aledsage commented 10 years ago

@grkvlt thanks for comments - have incorporated those; will wait for buildhive to run again and then merge. If you get a chance to look at most recent two commits then that would be great.

grkvlt commented 10 years ago

@aledsage One last suggestion, changing get() to getUnchecked() and removing duplicated thread interrupt handler block, otherwise all good :frog:

The thread safety issue in terminate() looks to be present in most of the shutdown type methods, so this is another thing to defer. But I would definitely consider using compare-and-set guards and an atomic flag as good practice for all state variable in Brooklyn management services.

buildhive commented 10 years ago

Brooklyn Central » brooklyn #2334 SUCCESS This pull request looks good (what's this?)

aledsage commented 10 years ago

Thanks @grkvlt - made the getUnchecked change; merging now.

buildhive commented 10 years ago

Brooklyn Central » brooklyn #2339 FAILURE Looks like there's a problem with this pull request (what's this?)