Skip to content

Commit 3f57247

Browse files
cekiceki
committed
only one shutdown hooks allowed, locks during LoggerContext.stop operation, fixes LOGBACK-1551
Signed-off-by: Ceki Gulcu <[email protected]>
1 parent 1b7fe94 commit 3f57247

File tree

16 files changed

+315
-103
lines changed

16 files changed

+315
-103
lines changed

‎logback-classic/src/main/java/ch/qos/logback/classic/LoggerContext.java‎

Lines changed: 48 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
/**
22
* Logback: the reliable, generic, fast and flexible logging framework.
33
* Copyright (C) 1999-2015, QOS.ch. All rights reserved.
4-
*
4+
* <p>
55
* This program and the accompanying materials are dual-licensed under
66
* either the terms of the Eclipse Public License v1.0 as published by
77
* the Eclipse Foundation
8-
*
9-
* or (per the licensee's choosing)
10-
*
8+
* <p>
9+
* or (per the licensee's choosing)
10+
* <p>
1111
* under the terms of the GNU Lesser General Public License version 2.1
1212
* as published by the Free Software Foundation.
1313
*/
@@ -23,6 +23,7 @@
2323
import java.util.Map;
2424
import java.util.concurrent.ConcurrentHashMap;
2525
import java.util.concurrent.ScheduledFuture;
26+
import java.util.concurrent.locks.ReentrantLock;
2627

2728
import ch.qos.logback.classic.util.LogbackMDCAdapter;
2829
import ch.qos.logback.core.status.ErrorStatus;
@@ -57,7 +58,9 @@
5758
*/
5859
public class LoggerContext extends ContextBase implements ILoggerFactory, LifeCycle {
5960

60-
/** Default setting of packaging data in stack traces */
61+
/**
62+
* Default setting of packaging data in stack traces
63+
*/
6164
public static final boolean DEFAULT_PACKAGING_DATA = false;
6265

6366
final Logger root;
@@ -74,7 +77,6 @@ public class LoggerContext extends ContextBase implements ILoggerFactory, LifeCy
7477

7578
MDCAdapter mdcAdapter;
7679

77-
7880
private int maxCallerDataDepth = ClassicConstants.DEFAULT_MAX_CALLEDER_DATA_DEPTH;
7981

8082
int resetCount = 0;
@@ -91,6 +93,10 @@ public LoggerContext() {
9193
initEvaluatorMap();
9294
size = 1;
9395
this.frameworkPackages = new ArrayList<String>();
96+
// In 1.5.7, the stop() method assumes that at some point the context has been started
97+
// since earlier versions of logback did not mandate calling the start method
98+
// we need to call in the constructor
99+
this.start();
94100
}
95101

96102
void initEvaluatorMap() {
@@ -117,8 +123,6 @@ public void setName(String name) {
117123
updateLoggerContextVO();
118124
}
119125

120-
121-
122126
public final Logger getLogger(final Class<?> clazz) {
123127
return getLogger(clazz.getName());
124128
}
@@ -194,9 +198,7 @@ public Logger exists(String name) {
194198

195199
final void noAppenderDefinedWarning(final Logger logger) {
196200
if (noAppenderWarning++ == 0) {
197-
getStatusManager().add(new WarnStatus(
198-
"No appenders present in context [" + getName() + "] for logger [" + logger.getName() + "].",
199-
logger));
201+
getStatusManager().add(new WarnStatus("No appenders present in context [" + getName() + "] for logger [" + logger.getName() + "].", logger));
200202
}
201203
}
202204

@@ -219,17 +221,24 @@ public boolean isPackagingDataEnabled() {
219221
return packagingDataEnabled;
220222
}
221223

222-
private void cancelScheduledTasks() {
223-
for (ScheduledFuture<?> sf : scheduledFutures) {
224-
sf.cancel(false);
224+
void cancelScheduledTasks() {
225+
226+
try {
227+
configurationLock.lock();
228+
229+
for (ScheduledFuture<?> sf : scheduledFutures) {
230+
sf.cancel(false);
231+
}
232+
scheduledFutures.clear();
233+
} finally {
234+
configurationLock.unlock();
225235
}
226-
scheduledFutures.clear();
227236
}
228237

229238
private void resetStatusListenersExceptResetResistant() {
230239
StatusManager sm = getStatusManager();
231240
for (StatusListener sl : sm.getCopyOfStatusListenerList()) {
232-
if(!sl.isResetResistant()) {
241+
if (!sl.isResetResistant()) {
233242
sm.remove(sl);
234243
}
235244
}
@@ -254,29 +263,28 @@ public void resetTurboFilterList() {
254263
turboFilterList.clear();
255264
}
256265

257-
final FilterReply getTurboFilterChainDecision_0_3OrMore(final Marker marker, final Logger logger, final Level level,
258-
final String format, final Object[] params, final Throwable t) {
266+
final FilterReply getTurboFilterChainDecision_0_3OrMore(final Marker marker, final Logger logger, final Level level, final String format,
267+
final Object[] params, final Throwable t) {
259268
if (turboFilterList.size() == 0) {
260269
return FilterReply.NEUTRAL;
261270
}
262271
return turboFilterList.getTurboFilterChainDecision(marker, logger, level, format, params, t);
263272
}
264273

265-
final FilterReply getTurboFilterChainDecision_1(final Marker marker, final Logger logger, final Level level,
266-
final String format, final Object param, final Throwable t) {
274+
final FilterReply getTurboFilterChainDecision_1(final Marker marker, final Logger logger, final Level level, final String format, final Object param,
275+
final Throwable t) {
267276
if (turboFilterList.size() == 0) {
268277
return FilterReply.NEUTRAL;
269278
}
270279
return turboFilterList.getTurboFilterChainDecision(marker, logger, level, format, new Object[] { param }, t);
271280
}
272281

273-
final FilterReply getTurboFilterChainDecision_2(final Marker marker, final Logger logger, final Level level,
274-
final String format, final Object param1, final Object param2, final Throwable t) {
282+
final FilterReply getTurboFilterChainDecision_2(final Marker marker, final Logger logger, final Level level, final String format, final Object param1,
283+
final Object param2, final Throwable t) {
275284
if (turboFilterList.size() == 0) {
276285
return FilterReply.NEUTRAL;
277286
}
278-
return turboFilterList.getTurboFilterChainDecision(marker, logger, level, format,
279-
new Object[] { param1, param2 }, t);
287+
return turboFilterList.getTurboFilterChainDecision(marker, logger, level, format, new Object[] { param1, param2 }, t);
280288
}
281289

282290
// === start listeners ==============================================
@@ -340,10 +348,21 @@ public void start() {
340348
}
341349

342350
public void stop() {
343-
reset();
344-
fireOnStop();
345-
resetAllListeners();
346-
super.stop();
351+
if (!isStarted())
352+
return;
353+
354+
try {
355+
configurationLock.lock();
356+
if (!isStarted())
357+
return;
358+
359+
reset();
360+
fireOnStop();
361+
resetAllListeners();
362+
super.stop();
363+
} finally {
364+
configurationLock.unlock();
365+
}
347366
}
348367

349368
/**
@@ -395,7 +414,6 @@ public List<String> getFrameworkPackages() {
395414
return frameworkPackages;
396415
}
397416

398-
399417
@Override
400418
public void setSequenceNumberGenerator(SequenceNumberGenerator sng) {
401419
this.sequenceNumberGenerator = sng;
@@ -411,7 +429,7 @@ public MDCAdapter getMDCAdapter() {
411429
}
412430

413431
public void setMDCAdapter(MDCAdapter anAdapter) {
414-
if(this.mdcAdapter != null) {
432+
if (this.mdcAdapter != null) {
415433
StatusManager sm = getStatusManager();
416434
sm.add(new WarnStatus("mdcAdapter being reset a second time", this));
417435
}

‎logback-classic/src/main/java/ch/qos/logback/classic/joran/ReconfigureOnChangeTask.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
import ch.qos.logback.core.status.StatusUtil;
3131

3232
import static ch.qos.logback.core.spi.ConfigurationEvent.newConfigurationChangeDetectorRunningEvent;
33-
import static ch.qos.logback.core.spi.ConfigurationEvent.newConfigurationEndedEvent;
33+
import static ch.qos.logback.core.spi.ConfigurationEvent.newConfigurationEndedSuccessfullyEvent;
3434

3535
public class ReconfigureOnChangeTask extends ContextAwareBase implements Runnable {
3636

@@ -92,6 +92,7 @@ private void performXMLConfiguration(LoggerContext lc, URL mainConfigurationURL)
9292
StatusUtil statusUtil = new StatusUtil(context);
9393
Model failsafeTop = jc.recallSafeConfiguration();
9494
URL mainURL = ConfigurationWatchListUtil.getMainWatchURL(context);
95+
addInfo("Resetting loggerContext ["+lc.getName()+"]");
9596
lc.reset();
9697
long threshold = System.currentTimeMillis();
9798
try {
@@ -129,8 +130,7 @@ private void fallbackConfiguration(LoggerContext lc, Model failsafeTop, URL main
129130
joranConfigurator.processModel(failsafeTop);
130131
addInfo(RE_REGISTERING_PREVIOUS_SAFE_CONFIGURATION);
131132
joranConfigurator.registerSafeConfiguration(failsafeTop);
132-
context.fireConfigurationEvent(newConfigurationEndedEvent(this));
133-
addInfo("after registerSafeConfiguration");
133+
context.fireConfigurationEvent(newConfigurationEndedSuccessfullyEvent(this));
134134
} catch (Exception e) {
135135
addError("Unexpected exception thrown by a configuration considered safe.", e);
136136
}

‎logback-classic/src/main/java/ch/qos/logback/classic/joran/SerializedModelConfigurator.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import java.io.InputStream;
3838
import java.net.MalformedURLException;
3939
import java.net.URL;
40+
import java.util.concurrent.locks.ReentrantLock;
4041

4142
import static ch.qos.logback.core.CoreConstants.MODEL_CONFIG_FILE_EXTENSION;
4243

@@ -81,8 +82,12 @@ private void configureByResource(URL url) {
8182
mc2mhl.link(defaultProcessor);
8283

8384
// disallow simultaneous configurations of the same context
84-
synchronized (context.getConfigurationLock()) {
85+
ReentrantLock configurationLock = context.getConfigurationLock();
86+
try {
87+
configurationLock.lock();
8588
defaultProcessor.process(model);
89+
} finally {
90+
configurationLock.unlock();
8691
}
8792
} else {
8893
throw new LogbackException(

‎logback-classic/src/test/java/ch/qos/logback/classic/Logback1551.java‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/*
2+
* Logback: the reliable, generic, fast and flexible logging framework.
3+
* Copyright (C) 1999-2024, QOS.ch. All rights reserved.
4+
*
5+
* This program and the accompanying materials are dual-licensed under
6+
* either the terms of the Eclipse Public License v1.0 as published by
7+
* the Eclipse Foundation
8+
*
9+
* or (per the licensee's choosing)
10+
*
11+
* under the terms of the GNU Lesser General Public License version 2.1
12+
* as published by the Free Software Foundation.
13+
*/
14+
15+
package ch.qos.logback.classic;
16+
17+
import ch.qos.logback.core.util.Duration;
18+
import org.junit.jupiter.api.BeforeEach;
19+
import org.junit.jupiter.api.Test;
20+
21+
import java.util.Arrays;
22+
import java.util.concurrent.ScheduledExecutorService;
23+
import java.util.concurrent.ScheduledFuture;
24+
import java.util.concurrent.TimeUnit;
25+
26+
public class Logback1551 {
27+
LoggerContext lc;
28+
29+
@BeforeEach
30+
public void setUp() throws Exception {
31+
lc = new LoggerContext();
32+
lc.setName("x");
33+
}
34+
@Test
35+
public void testConcurrentModificationScheduledTasks() {
36+
ScheduledExecutorService scheduledExecutorService = lc.getScheduledExecutorService();
37+
Duration duration = Duration.buildByMilliseconds(10);
38+
39+
Runnable runnable = new Runnable() {
40+
public void run() {
41+
try {
42+
Thread.sleep(100);
43+
} catch (InterruptedException e) {
44+
throw new RuntimeException(e);
45+
}
46+
}
47+
};
48+
ScheduledFuture<?> scheduledFuture = scheduledExecutorService.scheduleAtFixedRate(runnable,
49+
duration.getMilliseconds(), duration.getMilliseconds(), TimeUnit.MILLISECONDS);
50+
51+
lc.addScheduledFuture(scheduledFuture);
52+
int THREAD_COUNT = 20;
53+
Thread[] threads = new Thread[THREAD_COUNT];
54+
55+
for (int i = 0; i < THREAD_COUNT; i++) {
56+
threads[i] = new Thread(new CancelRunnable(lc));
57+
threads[i].start();
58+
}
59+
60+
Arrays.stream(threads).forEach(t-> {
61+
try {
62+
t.join();
63+
} catch (InterruptedException e) {
64+
throw new RuntimeException(e);
65+
}
66+
});
67+
68+
}
69+
70+
private class CancelRunnable implements Runnable {
71+
LoggerContext lc;
72+
public CancelRunnable(LoggerContext lc) {
73+
this.lc = lc;
74+
}
75+
76+
@Override
77+
public void run() {
78+
lc.cancelScheduledTasks();
79+
}
80+
}
81+
}

‎logback-classic/src/test/java/ch/qos/logback/classic/LoggerContextTest.java‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,30 @@
11
/**
22
* Logback: the reliable, generic, fast and flexible logging framework.
33
* Copyright (C) 1999-2015, QOS.ch. All rights reserved.
4-
*
4+
* <p>
55
* This program and the accompanying materials are dual-licensed under
66
* either the terms of the Eclipse Public License v1.0 as published by
77
* the Eclipse Foundation
8-
*
9-
* or (per the licensee's choosing)
10-
*
8+
* <p>
9+
* or (per the licensee's choosing)
10+
* <p>
1111
* under the terms of the GNU Lesser General Public License version 2.1
1212
* as published by the Free Software Foundation.
1313
*/
1414
package ch.qos.logback.classic;
1515

16-
import static ch.qos.logback.core.CoreConstants.FA_FILENAME_COLLISION_MAP;
17-
import static org.junit.jupiter.api.Assertions.assertEquals;
18-
import static org.junit.jupiter.api.Assertions.assertFalse;
19-
import static org.junit.jupiter.api.Assertions.assertNotNull;
20-
import static org.junit.jupiter.api.Assertions.assertNull;
21-
import static org.junit.jupiter.api.Assertions.assertTrue;
22-
import static org.junit.jupiter.api.Assertions.fail;
23-
24-
import java.util.Map;
25-
26-
import org.junit.jupiter.api.BeforeEach;
27-
2816
import ch.qos.logback.classic.turbo.NOPTurboFilter;
2917
import ch.qos.logback.core.CoreConstants;
3018
import ch.qos.logback.core.rolling.helper.FileNamePattern;
3119
import ch.qos.logback.core.status.StatusManager;
20+
import org.junit.jupiter.api.BeforeEach;
3221
import org.junit.jupiter.api.Test;
3322

23+
import java.util.Map;
24+
25+
import static ch.qos.logback.core.CoreConstants.FA_FILENAME_COLLISION_MAP;
26+
import static org.junit.jupiter.api.Assertions.*;
27+
3428
public class LoggerContextTest {
3529
LoggerContext lc;
3630

@@ -251,8 +245,8 @@ public void collisionMapsPostReset() {
251245
assertNotNull(fileCollisions);
252246
assertTrue(fileCollisions.isEmpty());
253247

254-
Map<String, FileNamePattern> filenamePatternCollisionMap = (Map<String, FileNamePattern>) lc
255-
.getObject(CoreConstants.RFA_FILENAME_PATTERN_COLLISION_MAP);
248+
Map<String, FileNamePattern> filenamePatternCollisionMap = (Map<String, FileNamePattern>) lc.getObject(
249+
CoreConstants.RFA_FILENAME_PATTERN_COLLISION_MAP);
256250
assertNotNull(filenamePatternCollisionMap);
257251
assertTrue(filenamePatternCollisionMap.isEmpty());
258252
}

‎logback-classic/src/test/java/ch/qos/logback/classic/joran/ChangeDetectedListener.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class ChangeDetectedListener implements ConfigurationEventListener {
3333
public void listen(ConfigurationEvent configurationEvent) {
3434
switch (configurationEvent.getEventType()) {
3535
case CHANGE_DETECTED:
36-
System.out.println(this.toString() + "#listen Change detected" + " count="+countDownLatch.getCount());
36+
System.out.println(this.toString() + "#listen Change detected " + configurationEvent +" count="+countDownLatch.getCount());
3737

3838
countDownLatch.countDown();
3939
Object data = configurationEvent.getData();

0 commit comments

Comments
 (0)