explicitly declare `Map<?,?>` in more places to prevent some javac's from getting confused about how to call super (which can cause stack overflow, cf https://github.com/apache/incubator-brooklyn/pull/489 ).
also introduce some help for identifying recursive calls; no longer used in the codebase, but will be useful if we need to track recursive calls again, and it is tested. Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/11972022 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/11972022 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/11972022 Branch: refs/heads/master Commit: 119720226bf9fef8cc987d98361189c1dc349d33 Parents: e8a7072 Author: Alex Heneveld <[email protected]> Authored: Thu Jan 29 14:19:03 2015 +0000 Committer: Alex Heneveld <[email protected]> Committed: Thu Jan 29 17:02:49 2015 +0000 ---------------------------------------------------------------------- .../AggregatingMachineProvisioningLocation.java | 2 +- .../FixedListMachineProvisioningLocation.java | 2 +- .../LocalhostMachineProvisioningLocation.java | 4 +-- .../location/basic/SshMachineLocation.java | 4 ++- .../location/jclouds/JcloudsLocation.java | 2 +- .../util/javalang/StackTraceSimplifier.java | 25 +++++++++++++- .../util/javalang/StackTraceSimplifierTest.java | 36 +++++++++++++++++++- 7 files changed, 67 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/11972022/core/src/main/java/brooklyn/location/basic/AggregatingMachineProvisioningLocation.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/location/basic/AggregatingMachineProvisioningLocation.java b/core/src/main/java/brooklyn/location/basic/AggregatingMachineProvisioningLocation.java index dbb52c5..a644459 100644 --- a/core/src/main/java/brooklyn/location/basic/AggregatingMachineProvisioningLocation.java +++ b/core/src/main/java/brooklyn/location/basic/AggregatingMachineProvisioningLocation.java @@ -78,7 +78,7 @@ public class AggregatingMachineProvisioningLocation<T extends MachineLocation> e } @Override - public AbstractLocation configure(Map properties) { + public AbstractLocation configure(Map<?,?> properties) { if (lock == null) { lock = new Object(); provisioners = Lists.<MachineProvisioningLocation<T>>newArrayList(); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/11972022/core/src/main/java/brooklyn/location/basic/FixedListMachineProvisioningLocation.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/location/basic/FixedListMachineProvisioningLocation.java b/core/src/main/java/brooklyn/location/basic/FixedListMachineProvisioningLocation.java index 263e307..97f1117 100644 --- a/core/src/main/java/brooklyn/location/basic/FixedListMachineProvisioningLocation.java +++ b/core/src/main/java/brooklyn/location/basic/FixedListMachineProvisioningLocation.java @@ -121,7 +121,7 @@ implements MachineProvisioningLocation<T>, Closeable { } @Override - public AbstractLocation configure(Map properties) { + public AbstractLocation configure(Map<?,?> properties) { if (machines == null) machines = Sets.newLinkedHashSet(); if (inUse == null) inUse = Sets.newLinkedHashSet(); if (pendingRemoval == null) pendingRemoval = Sets.newLinkedHashSet(); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/11972022/core/src/main/java/brooklyn/location/basic/LocalhostMachineProvisioningLocation.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/location/basic/LocalhostMachineProvisioningLocation.java b/core/src/main/java/brooklyn/location/basic/LocalhostMachineProvisioningLocation.java index 324f5a0..051b37e 100644 --- a/core/src/main/java/brooklyn/location/basic/LocalhostMachineProvisioningLocation.java +++ b/core/src/main/java/brooklyn/location/basic/LocalhostMachineProvisioningLocation.java @@ -122,7 +122,7 @@ public class LocalhostMachineProvisioningLocation extends FixedListMachineProvis return LocationSpec.create(LocalhostMachineProvisioningLocation.class); } - public LocalhostMachineProvisioningLocation configure(Map flags) { + public LocalhostMachineProvisioningLocation configure(Map<?,?> flags) { super.configure(flags); if (!truth(getDisplayName())) { setDisplayName("localhost"); } @@ -294,7 +294,7 @@ public class LocalhostMachineProvisioningLocation extends FixedListMachineProvis } @Override - public LocalhostMachine configure(Map properties) { + public LocalhostMachine configure(Map<?,?> properties) { if (address==null || !properties.containsKey("address")) address = Networking.getLocalHost(); super.configure(properties); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/11972022/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java b/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java index 2b7d8b1..4a165f2 100644 --- a/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java +++ b/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java @@ -35,6 +35,7 @@ import java.io.StringReader; import java.net.InetAddress; import java.net.Socket; import java.security.KeyPair; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -77,6 +78,7 @@ import brooklyn.util.internal.ssh.ShellTool; import brooklyn.util.internal.ssh.SshException; import brooklyn.util.internal.ssh.SshTool; import brooklyn.util.internal.ssh.sshj.SshjTool; +import brooklyn.util.javalang.StackTraceSimplifier; import brooklyn.util.mutex.MutexSupport; import brooklyn.util.mutex.WithMutexes; import brooklyn.util.net.Urls; @@ -334,7 +336,7 @@ public class SshMachineLocation extends AbstractLocation implements MachineLocat } @Override - public SshMachineLocation configure(Map properties) { + public SshMachineLocation configure(Map<?,?> properties) { super.configure(properties); // TODO Note that check for addresss!=null is done automatically in super-constructor, in FlagUtils.checkRequiredFields http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/11972022/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java index cafe1c7..b10973d 100644 --- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java +++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java @@ -221,7 +221,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im } @Override - public JcloudsLocation configure(Map properties) { + public JcloudsLocation configure(Map<?,?> properties) { super.configure(properties); if (getLocalConfigBag().containsKey("providerLocationId")) { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/11972022/utils/common/src/main/java/brooklyn/util/javalang/StackTraceSimplifier.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/brooklyn/util/javalang/StackTraceSimplifier.java b/utils/common/src/main/java/brooklyn/util/javalang/StackTraceSimplifier.java index 2247136..283cb98 100644 --- a/utils/common/src/main/java/brooklyn/util/javalang/StackTraceSimplifier.java +++ b/utils/common/src/main/java/brooklyn/util/javalang/StackTraceSimplifier.java @@ -22,6 +22,7 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.util.Arrays; import java.util.Collection; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; @@ -30,6 +31,7 @@ import org.slf4j.LoggerFactory; import brooklyn.util.text.Strings; +import com.google.common.annotations.Beta; import com.google.common.collect.ImmutableSet; /** @@ -176,5 +178,26 @@ public class StackTraceSimplifier { t.printStackTrace(pw); return sw.getBuffer().toString(); } - + + /** returns the number of times the calling method occurs elsewhere in the stack trace; + * 0 if no recursion, 1 if it has cycled three times, etc. */ + @Beta // useful to track down things like https://github.com/apache/incubator-brooklyn/pull/489 + public static int getRecursiveCallCount() { + StackTraceElement[] t = cleanStackTrace(new Throwable().getStackTrace()); + Iterator<StackTraceElement> ti = Arrays.asList(t).iterator(); + ti.next(); + if (!ti.hasNext()) return 0; + // t0 is the caller + StackTraceElement t0 = ti.next(); + String l0 = t0.getClassName()+"."+t0.getMethodName()+"("+t0.getFileName()+")"; + int count = 0; + while (ti.hasNext()) { + StackTraceElement ta = ti.next(); + String li = ta.getClassName()+"."+ta.getMethodName()+"("+ta.getFileName()+")"; + // if we have something in a different method, then something back in the method + // from which the recursive check came, then return true + if (li.equals(l0)) count++; + } + return count; + } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/11972022/utils/common/src/test/java/brooklyn/util/javalang/StackTraceSimplifierTest.java ---------------------------------------------------------------------- diff --git a/utils/common/src/test/java/brooklyn/util/javalang/StackTraceSimplifierTest.java b/utils/common/src/test/java/brooklyn/util/javalang/StackTraceSimplifierTest.java index 3ed7c2b..1c3c5ca 100644 --- a/utils/common/src/test/java/brooklyn/util/javalang/StackTraceSimplifierTest.java +++ b/utils/common/src/test/java/brooklyn/util/javalang/StackTraceSimplifierTest.java @@ -21,6 +21,8 @@ package brooklyn.util.javalang; import org.testng.Assert; import org.testng.annotations.Test; +import brooklyn.util.exceptions.Exceptions; + public class StackTraceSimplifierTest { @Test @@ -44,5 +46,37 @@ public class StackTraceSimplifierTest { Assert.assertTrue(t.getStackTrace()[0].getClassName().startsWith("org.testng"), "trace was: "+t.getStackTrace()[0]); } - + + private int m1(int x) { + int count = StackTraceSimplifier.getRecursiveCallCount(); + if (count>100) throw new RuntimeException("expected"); + if (x<=0) { + return count; + } + return m2(x-1); + } + private int m2(int x) { + if (x<=0) return -1; + return m1(x-1); + } + @Test + public void testIsRecursiveCallToSelf() { + Assert.assertEquals(m1(1), -1); + Assert.assertEquals(m2(1), 0); + Assert.assertEquals(m1(2), 1); + Assert.assertEquals(m2(2), -1); + Assert.assertEquals(m1(3), -1); + Assert.assertEquals(m1(4), 2); + Assert.assertEquals(m1(20), 10); + + try { + m1(500); + Assert.fail("should have failed on recursive call"); + } catch (Exception e) { + Exceptions.propagateIfFatal(e); + if (!e.getMessage().equals("expected")) + throw Exceptions.propagate(e); + } + } + }
