From d09aa6cd1efdb8733343781be86b11d73e5bc1ae Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Tue, 7 Apr 2015 14:26:35 -0400 Subject: [PATCH] Fix: Java agent: update ref count in enabledLoggers Integer objects are immutable in Java, so Integer refcount = enabledLoggers.get(name); refcount--; does not update the value in enabledLoggers. However, this bug "fixes" a bug in the session daemon (LTTng-tools) which sends multiple _disable event_ commands for the same event name (one when the event is disabled manually, and another one when the session is destroyed). Therefore we use the registration done command's version to know if the Java agent is connected to a fixed or non-fixed session daemon, using the old behaviour if connected to a non-fixed one. Signed-off-by: Philippe Proulx Signed-off-by: Mathieu Desnoyers --- .../ust/agent/LTTngTCPSessiondClient.java | 16 ++++++++++++++ .../org/lttng/ust/agent/LogFramework.java | 1 + .../lttng/ust/agent/LogFrameworkSkeleton.java | 22 +++++++++++++++++++ 3 files changed, 39 insertions(+) diff --git a/liblttng-ust-java-agent/java/org/lttng/ust/agent/LTTngTCPSessiondClient.java b/liblttng-ust-java-agent/java/org/lttng/ust/agent/LTTngTCPSessiondClient.java index db4e1f70..76e8426b 100644 --- a/liblttng-ust-java-agent/java/org/lttng/ust/agent/LTTngTCPSessiondClient.java +++ b/liblttng-ust-java-agent/java/org/lttng/ust/agent/LTTngTCPSessiondClient.java @@ -185,6 +185,22 @@ class LTTngTCPSessiondClient implements Runnable { switch (headerCmd.cmd) { case CMD_REG_DONE: { + /* + * Check command version: + * + * * 0: Connected to a non-fixed session daemon, + * which could send multiple disable + * event commands: do not decrement + * reference count on disable event command + * (original behaviour). + * * >0: Connected to a fixed session daemon: + * do decrement reference count on disable + * event command. + */ + if (headerCmd.cmd_version > 0) { + this.log.setEnableRefCountDecrement(true); + } + /* * Release semaphore so meaning registration is done and we * can proceed to continue tracing. diff --git a/liblttng-ust-java-agent/java/org/lttng/ust/agent/LogFramework.java b/liblttng-ust-java-agent/java/org/lttng/ust/agent/LogFramework.java index 0dd7c982..692a8202 100644 --- a/liblttng-ust-java-agent/java/org/lttng/ust/agent/LogFramework.java +++ b/liblttng-ust-java-agent/java/org/lttng/ust/agent/LogFramework.java @@ -26,4 +26,5 @@ interface LogFramework { Iterator listLoggers(); Boolean isRoot(); void reset(); + void setEnableRefCountDecrement(boolean enableRefCountDecrement); } diff --git a/liblttng-ust-java-agent/java/org/lttng/ust/agent/LogFrameworkSkeleton.java b/liblttng-ust-java-agent/java/org/lttng/ust/agent/LogFrameworkSkeleton.java index 4ad981e9..6775827a 100644 --- a/liblttng-ust-java-agent/java/org/lttng/ust/agent/LogFrameworkSkeleton.java +++ b/liblttng-ust-java-agent/java/org/lttng/ust/agent/LogFrameworkSkeleton.java @@ -27,6 +27,19 @@ public abstract class LogFrameworkSkeleton implements LogFramework { /* A map of event name and reference count */ private Map enabledLoggers; + /* + * If the following attribute is false, the internal ref count is + * never decremented when disabling a logger. This was the original + * behaviour of this agent, and this bug worked in concert with a + * bug in the session daemon which would send multiple disable + * commands for the same event name (manual disable + another + * disable on session destroy). The following attribute is needed + * because this version of the agent could be connected to a + * fixed session daemon, or a non-fixed session daemon, and it needs + * to work in both situations. + */ + private boolean enableRefCountDecrement = false; + public LogFrameworkSkeleton() { this.enabledLoggers = new HashMap(); } @@ -68,6 +81,11 @@ public abstract class LogFrameworkSkeleton implements LogFramework { refcount--; assert (refcount >= 0); + if (enableRefCountDecrement) { + /* Effectively decrement reference count. */ + enabledLoggers.put(name, refcount); + } + if (refcount == 0) { /* Event is not used anymore, remove it from the map */ Integer oldval = enabledLoggers.remove(name); @@ -91,4 +109,8 @@ public abstract class LogFrameworkSkeleton implements LogFramework { protected Integer getEventCount() { return enabledLoggers.size(); } + + public void setEnableRefCountDecrement(boolean enableRefCountDecrement) { + this.enableRefCountDecrement = enableRefCountDecrement; + } } -- 2.34.1