Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/617#discussion_r30589991
--- Diff:
usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java ---
@@ -126,45 +126,62 @@ public void shutdown(final boolean stopAppsFirst,
final boolean forceShutdownOnE
new Thread("shutdown") {
public void run() {
- if (stopAppsFirst) {
- CountdownTimer shutdownTimeoutTimer = null;
- if (!shutdownTimeout.equals(Duration.ZERO)) {
- shutdownTimeoutTimer =
shutdownTimeout.countdownTimer();
- }
+ boolean terminateTried = false;
+ try {
+ if (stopAppsFirst) {
+ CountdownTimer shutdownTimeoutTimer = null;
+ if (!shutdownTimeout.equals(Duration.ZERO)) {
+ shutdownTimeoutTimer =
shutdownTimeout.countdownTimer();
+ }
- List<Task<?>> stoppers = new ArrayList<Task<?>>();
- for (Application app: mgmt().getApplications()) {
- if (app instanceof StartableApplication)
-
stoppers.add(Entities.invokeEffector((EntityLocal)app, app,
StartableApplication.STOP));
- }
+ List<Task<?>> stoppers = new ArrayList<Task<?>>();
+ for (Application app: mgmt().getApplications()) {
+ if (app instanceof StartableApplication)
+
stoppers.add(Entities.invokeEffector((EntityLocal)app, app,
StartableApplication.STOP));
+ }
- try {
for (Task<?> t: stoppers) {
if (!waitAppShutdown(shutdownTimeoutTimer, t))
{
//app stop error
hasAppErrorsOrTimeout.set(true);
}
}
- } catch (TimeoutException e) {
+ }
+
+ terminateTried = true;
+ ((ManagementContextInternal)mgmt()).terminate();
+
+ } catch (Throwable e) {
+ Throwable interesting =
Exceptions.getFirstInteresting(e);
+ if (interesting instanceof TimeoutException) {
//timeout while waiting for apps to stop
+ log.warn("Timeout shutting down:
"+Exceptions.collapseText(e));
+ log.debug("Timeout shutting down: "+e, e);
hasAppErrorsOrTimeout.set(true);
+
+ } else {
+ // swallow fatal, so we notify the outer loop to
continue with shutdown
+ log.error("Unexpected error shutting down:
"+Exceptions.collapseText(e), e);
+
}
-
- if (hasAppErrorsOrTimeout.get() &&
!forceShutdownOnError) {
- complete();
- //There are app errors, don't exit the process.
- return;
+ hasAppErrorsOrTimeout.set(true);
+
+ if (!terminateTried) {
+ ((ManagementContextInternal)mgmt()).terminate();
}
+ } finally {
+
+ complete();
+
+ if (!hasAppErrorsOrTimeout.get() ||
forceShutdownOnError) {
+ //give the http request a chance to complete
gracefully
+ Time.sleep(delayForHttpReturn);
+ System.exit(0);
+ }
+
+ // There are app errors, don't exit the process,
allowing any exception to continue throwing
+ log.warn("Abandoning shutdown because there were
errors and shutdown was not forced.");
--- End diff --
personal preference to put this kind of thing in an else block (even though
the above block will have done `System.exit`, so can only reach here if we
didn't go into the if).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
