enabledEventList is shared between the LTTngThread and eventTimer threads but is not synchronized.
This patch changes enabledEventList's type from an ArrayList to a synchronized HashSet. Signed-off-by: Jérémie Galarneau <[email protected]> --- .../org/lttng/ust/jul/LTTngTCPSessiondClient.java | 129 ++++++++++++--------- 1 file changed, 74 insertions(+), 55 deletions(-) diff --git a/liblttng-ust-jul/org/lttng/ust/jul/LTTngTCPSessiondClient.java b/liblttng-ust-jul/org/lttng/ust/jul/LTTngTCPSessiondClient.java index 25d1cb2..35f768f 100644 --- a/liblttng-ust-jul/org/lttng/ust/jul/LTTngTCPSessiondClient.java +++ b/liblttng-ust-jul/org/lttng/ust/jul/LTTngTCPSessiondClient.java @@ -30,10 +30,14 @@ import java.net.*; import java.lang.management.ManagementFactory; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; import java.util.List; +import java.util.Set; import java.util.Timer; import java.util.TimerTask; import java.util.logging.Logger; +import java.util.Collections; class USTRegisterMsg { public static int pid; @@ -57,7 +61,8 @@ public class LTTngTCPSessiondClient { private Semaphore registerSem; private Timer eventTimer; - private List<LTTngEvent> enabledEventList = new ArrayList<LTTngEvent>(); + private Set<LTTngEvent> enabledEventSet = + Collections.synchronizedSet(new HashSet<LTTngEvent>()); /* * Map of Logger objects that have been enabled. They are indexed by name. */ @@ -82,65 +87,81 @@ public class LTTngTCPSessiondClient { this.eventTimer.scheduleAtFixedRate(new TimerTask() { @Override public void run() { - /* - * We have to make a copy here since it is possible that the - * enabled event list is changed during an iteration on it. - */ - List<LTTngEvent> tmpList = new ArrayList<LTTngEvent>(enabledEventList); - - LTTngSessiondCmd2_4.sessiond_enable_handler enableCmd = new - LTTngSessiondCmd2_4.sessiond_enable_handler(); - for (LTTngEvent event: tmpList) { - int ret; - Logger logger; - + synchronized (enabledEventSet) { + LTTngSessiondCmd2_4.sessiond_enable_handler enableCmd = new + LTTngSessiondCmd2_4.sessiond_enable_handler(); /* - * Check if this Logger name has been enabled already. Note - * that in the case of "*", it's never added in that hash - * table thus the enable command does a lookup for each - * logger name in that hash table for the * case in order - * to make sure we don't enable twice the same logger - * because JUL apparently accepts that the *same* - * LogHandler can be added twice on a Logger object... - * don't ask... + * Modifying events in a Set will raise a + * ConcurrentModificationException. Thus, we remove an event + * and add its modified version to modifiedEvents when a + * modification is necessary. */ - logger = enabledLoggers.get(event.name); - if (logger != null) { - continue; - } + Set<LTTngEvent> modifiedEvents = new HashSet<LTTngEvent>(); + Iterator<LTTngEvent> it = enabledEventSet.iterator(); - /* - * Set to one means that the enable all event has been seen - * thus event from that point on must use loglevel for all - * events. Else the object has its own loglevel. - */ - if (handler.logLevelUseAll == 1) { - event.logLevel.level = handler.logLevelAll; - event.logLevel.type = handler.logLevelTypeAll; - } + while (it.hasNext()) { + int ret; + Logger logger; + LTTngEvent event = it.next(); - /* - * The all event is a special case since we have to iterate - * over every Logger to see which one was not enabled. - */ - if (event.name.equals("*")) { - enableCmd.name = event.name; - enableCmd.lttngLogLevel = event.logLevel.level; - enableCmd.lttngLogLevelType = event.logLevel.type; /* - * The return value is irrelevant since the * event is - * always kept in the list. + * Check if this Logger name has been enabled already. Note + * that in the case of "*", it's never added in that hash + * table thus the enable command does a lookup for each + * logger name in that hash table for the * case in order + * to make sure we don't enable twice the same logger + * because JUL apparently accepts that the *same* + * LogHandler can be added twice on a Logger object... + * don't ask... */ - enableCmd.execute(handler, enabledLoggers); - continue; - } + logger = enabledLoggers.get(event.name); + if (logger != null) { + continue; + } - ret = enableCmd.enableLogger(handler, event, enabledLoggers); - if (ret == 1) { - /* Enabled so remove the event from the list. */ - enabledEventList.remove(event); + /* + * Set to one means that the enable all event has been seen + * thus event from that point on must use loglevel for all + * events. Else the object has its own loglevel. + */ + if (handler.logLevelUseAll == 1) { + it.remove(); + event.logLevel.level = handler.logLevelAll; + event.logLevel.type = handler.logLevelTypeAll; + modifiedEvents.add(event); + } + + /* + * The all event is a special case since we have to iterate + * over every Logger to see which one was not enabled. + */ + if (event.name.equals("*")) { + enableCmd.name = event.name; + enableCmd.lttngLogLevel = event.logLevel.level; + enableCmd.lttngLogLevelType = event.logLevel.type; + /* + * The return value is irrelevant since the * event is + * always kept in the set. + */ + enableCmd.execute(handler, enabledLoggers); + continue; + } + + ret = enableCmd.enableLogger(handler, event, enabledLoggers); + if (ret == 1) { + /* Enabled so remove the event from the set. */ + if (!modifiedEvents.remove(event)) { + /* + * event can only be present in one of + * the sets. + */ + it.remove(); + } + } } + enabledEventSet.addAll(modifiedEvents); } + } }, this.timerDelay, this.timerDelay); @@ -274,12 +295,10 @@ public class LTTngTCPSessiondClient { event = enableCmd.execute(this.handler, this.enabledLoggers); if (event != null) { /* - * Add the event to the list so it can be enabled if + * Add the event to the set so it can be enabled if * the logger appears at some point in time. */ - if (enabledEventList.contains(event) == false) { - enabledEventList.add(event); - } + enabledEventSet.add(event); } data = enableCmd.getBytes(); break; -- 1.9.0 _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
