You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2017/10/06 08:06:40 UTC
[16/23] brooklyn-server git commit: change when cancellation is done
for getImmediate - means effector invocation now works
change when cancellation is done for getImmediate - means effector invocation now works
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/bb26d320
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/bb26d320
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/bb26d320
Branch: refs/heads/master
Commit: bb26d320906ad86888c28cc32b2fa36709e70d1a
Parents: d7b086b
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Thu Sep 28 16:55:33 2017 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu Sep 28 17:29:59 2017 +0100
----------------------------------------------------------------------
.../util/core/task/BasicExecutionContext.java | 18 +++++++-----------
.../brooklyn/core/effector/EffectorSayHiTest.java | 6 +++---
2 files changed, 10 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb26d320/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java b/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
index f9f431a..2c8ddab 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
@@ -287,17 +287,6 @@ public class BasicExecutionContext extends AbstractExecutionContext {
try {
return runInSameThread(fakeTaskForContext, new Callable<Maybe<T>>() {
public Maybe<T> call() {
- // could try to make this work for more types of tasks by not cancelling, just interrupting;
- // however there is a danger that immediate-submission tasks are leaked if we don't cancel.
- // for instance with DSTs the thread interrupt may apply only to the main job queue.andWait blocking,
- // leaving other tasks leaked.
- //
- // this method is best-effort so fine if it doesn't succeed. good if we can expand
- // coverage but NOT at the expense of major leaks of course!
- //
- // see WIP test in EffectorSayHiTest
- fakeTaskForContext.cancel();
-
boolean wasAlreadyInterrupted = Thread.interrupted();
try {
return job.getImmediately();
@@ -305,6 +294,13 @@ public class BasicExecutionContext extends AbstractExecutionContext {
if (wasAlreadyInterrupted) {
Thread.currentThread().interrupt();
}
+ // we've acknowledged that getImmediate may wreck (cancel) the task,
+ // their first priority is to prevent them from leaking;
+ // however previously we did the cancel before running,
+ // doing it after means more tasks successfully execute
+ // (the interrupt is sufficient to prevent them blocking);
+ // see test EffectorSayHiTest.testInvocationGetImmediately
+ fakeTaskForContext.cancel();
}
} });
} catch (Exception e) {
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb26d320/core/src/test/java/org/apache/brooklyn/core/effector/EffectorSayHiTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/effector/EffectorSayHiTest.java b/core/src/test/java/org/apache/brooklyn/core/effector/EffectorSayHiTest.java
index bc41d45..e7dc626 100644
--- a/core/src/test/java/org/apache/brooklyn/core/effector/EffectorSayHiTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/effector/EffectorSayHiTest.java
@@ -61,6 +61,7 @@ public class EffectorSayHiTest extends BrooklynAppUnitTestSupport {
//TODO test edge/error conditions
//(missing parameters, wrong number of params, etc)
+ @SuppressWarnings("unused")
private static final Logger log = LoggerFactory.getLogger(EffectorSayHiTest.class);
private MyEntity e;
@@ -109,11 +110,10 @@ public class EffectorSayHiTest extends BrooklynAppUnitTestSupport {
.get( Effectors.invocation(e, MyEntity.SAY_HI_1, ImmutableMap.of("name", "Bob", "greeting", "hi")) ), "hi Bob");
}
- @Test(groups="WIP") // see comments at BasicExecutionContext.getImmediately
- // TODO this will be fixed soon by #835
+ @Test(invocationCount=100)
public void testInvocationGetImmediately() throws Exception {
assertEquals(((EntityInternal)e).getExecutionContext()
- .getImmediately( Effectors.invocation(e, MyEntity.SAY_HI_1, ImmutableMap.of("name", "Bob", "greeting", "hi")) ), "hi Bob");
+ .getImmediately( Effectors.invocation(e, MyEntity.SAY_HI_1, ImmutableMap.of("name", "Bob", "greeting", "hi")) ).get(), "hi Bob");
}
@Test