Skip to content

Commit 08d71b1

Browse files
cekiceki
committed
fix LOGBACK-1716 by adding a ThreadPoolExecutor in addition to the ScheduledExecutor in Context interface
Signed-off-by: Ceki Gulcu <[email protected]>
1 parent 4ba0594 commit 08d71b1

19 files changed

Lines changed: 83 additions & 88 deletions

File tree

‎logback-classic-blackbox/src/test/java/ch/qos/logback/classic/blackbox/net/SMTPAppender_GreenTest.java‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import java.io.ByteArrayOutputStream;
1818
import java.io.IOException;
1919
import java.io.InputStream;
20+
import java.util.concurrent.ExecutorService;
2021
import java.util.concurrent.TimeUnit;
2122

2223
import ch.qos.logback.classic.blackbox.BlackboxClassicTestConstants;
@@ -52,10 +53,7 @@
5253
import javax.mail.internet.MimeMessage;
5354
import javax.mail.internet.MimeMultipart;
5455

55-
import static org.junit.jupiter.api.Assertions.assertEquals;
56-
import static org.junit.jupiter.api.Assertions.assertFalse;
57-
import static org.junit.jupiter.api.Assertions.assertNotNull;
58-
import static org.junit.jupiter.api.Assertions.assertTrue;
56+
import static org.junit.jupiter.api.Assertions.*;
5957

6058
public class SMTPAppender_GreenTest {
6159

@@ -161,8 +159,14 @@ private MimeMultipart verifyAndExtractMimeMultipart(String subject)
161159
}
162160

163161
void waitUntilEmailIsSent() throws InterruptedException {
164-
loggerContext.getScheduledExecutorService().shutdown();
165-
loggerContext.getScheduledExecutorService().awaitTermination(1000, TimeUnit.MILLISECONDS);
162+
ExecutorService es = loggerContext.getExecutorService();
163+
es.shutdown();
164+
boolean terminated = es.awaitTermination(1000, TimeUnit.MILLISECONDS);
165+
// this assertion may be needlessly strict
166+
if(!terminated) {
167+
fail("executor elapsed before accorded delay");
168+
}
169+
166170
}
167171

168172
@Test

‎logback-classic/src/main/java/ch/qos/logback/classic/net/ReceiverBase.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public final void start() {
3636
throw new IllegalStateException("context not set");
3737
}
3838
if (shouldStart()) {
39-
getContext().getScheduledExecutorService().execute(getRunnableTask());
39+
getContext().getExecutorService().execute(getRunnableTask());
4040
started = true;
4141
}
4242
}

‎logback-classic/src/main/java/ch/qos/logback/classic/net/server/ServerSocketReceiver.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ protected boolean shouldStart() {
5858

5959
ServerListener<RemoteAppenderClient> listener = createServerListener(serverSocket);
6060

61-
runner = createServerRunner(listener, getContext().getScheduledExecutorService());
61+
runner = createServerRunner(listener, getContext().getExecutorService());
6262
runner.setContext(getContext());
6363
return true;
6464
} catch (Exception ex) {

‎logback-classic/src/main/java/ch/qos/logback/classic/turbo/ReconfigureOnChangeFilter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ private void updateMaskIfNecessary(long now) {
152152
// reader lock.
153153
void detachReconfigurationToNewThread() {
154154
addInfo("Detected change in [" + configurationWatchList.getCopyOfFileWatchList() + "]");
155-
context.getScheduledExecutorService().submit(new ReconfiguringThread());
155+
context.getExecutorService().submit(new ReconfiguringThread());
156156
}
157157

158158
void updateNextCheck(long now) {

‎logback-classic/src/test/java/ch/qos/logback/classic/issue/lbclassic323/Barebones.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@
1616
import ch.qos.logback.core.Context;
1717
import ch.qos.logback.core.ContextBase;
1818

19+
20+
// LOGBACK-329
1921
public class Barebones {
2022

2123
public static void main(String[] args) {
2224
Context context = new ContextBase();
2325
for (int i = 0; i < 3; i++) {
2426
SenderRunnable senderRunnable = new SenderRunnable("" + i);
25-
context.getScheduledExecutorService().execute(senderRunnable);
27+
context.getExecutorService().execute(senderRunnable);
2628
}
2729
System.out.println("done");
2830
// System.exit(0);

‎logback-classic/src/test/java/ch/qos/logback/classic/net/server/ServerSocketReceiverFunctionalTest.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626

2727
import org.junit.jupiter.api.AfterEach;
2828
import org.junit.jupiter.api.BeforeEach;
29-
import org.junit.jupiter.api.Disabled;
3029
import org.junit.jupiter.api.Test;
3130

3231
import ch.qos.logback.classic.Level;
@@ -45,7 +44,6 @@
4544
* interface, and validate that it receives messages and delivers them to its
4645
* appender.
4746
*/
48-
@Disabled
4947
public class ServerSocketReceiverFunctionalTest {
5048

5149
private static final int EVENT_COUNT = 10;
@@ -78,7 +76,7 @@ public void setUp() throws Exception {
7876
@AfterEach
7977
public void tearDown() throws Exception {
8078
receiver.stop();
81-
ExecutorService executor = lc.getScheduledExecutorService();
79+
ExecutorService executor = lc.getExecutorService();
8280
executor.shutdownNow();
8381
executor.awaitTermination(SHUTDOWN_DELAY, TimeUnit.MILLISECONDS);
8482
assertTrue(executor.isTerminated());

‎logback-core/src/main/java/ch/qos/logback/core/Context.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import java.util.concurrent.ExecutorService;
1818
import java.util.concurrent.ScheduledExecutorService;
1919
import java.util.concurrent.ScheduledFuture;
20+
import java.util.concurrent.ThreadPoolExecutor;
2021

2122
import ch.qos.logback.core.spi.ConfigurationEvent;
2223
import ch.qos.logback.core.spi.ConfigurationEventListener;
@@ -111,12 +112,11 @@ public interface Context extends PropertyContainer {
111112
/**
112113
* Returns the ScheduledExecutorService for this context.
113114
*
114-
* @return
115+
* @return ScheduledExecutorService
115116
* @since 1.1.7
116117
*/
117118
// Apparently ScheduledThreadPoolExecutor has limitation where a task cannot be
118-
// submitted from
119-
// within a running task. ThreadPoolExecutor does not have this limitation.
119+
// submitted from within a running task. ThreadPoolExecutor does not have this limitation.
120120
// This causes tests failures in
121121
// SocketReceiverTest.testDispatchEventForEnabledLevel and
122122
// ServerSocketReceiverFunctionalTest.testLogEventFromClient.
@@ -127,11 +127,12 @@ public interface Context extends PropertyContainer {
127127
* tasks in a separate thread.
128128
*
129129
* @return the executor for this context.
130-
* @since 1.0.0
131-
* @deprecated use {@link#getScheduledExecutorService()} instead
130+
* @since 1.0.00 (undeprecated in 1.4.7)
131+
*
132132
*/
133133
ExecutorService getExecutorService();
134134

135+
135136
/**
136137
* Register a component that participates in the context's life cycle.
137138
* <p>

‎logback-core/src/main/java/ch/qos/logback/core/ContextBase.java‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,7 @@
2222
import java.util.HashMap;
2323
import java.util.List;
2424
import java.util.Map;
25-
import java.util.concurrent.ConcurrentHashMap;
26-
import java.util.concurrent.CopyOnWriteArrayList;
27-
import java.util.concurrent.ExecutorService;
28-
import java.util.concurrent.ScheduledExecutorService;
29-
import java.util.concurrent.ScheduledFuture;
25+
import java.util.concurrent.*;
3026

3127
import ch.qos.logback.core.rolling.helper.FileNamePattern;
3228
import ch.qos.logback.core.spi.ConfigurationEvent;
@@ -56,6 +52,9 @@ public class ContextBase implements Context, LifeCycle {
5652
final private List<ConfigurationEventListener> configurationEventListenerList = new CopyOnWriteArrayList<>();
5753

5854
private ScheduledExecutorService scheduledExecutorService;
55+
56+
private ThreadPoolExecutor threadPoolExecutor;
57+
5958
protected List<ScheduledFuture<?>> scheduledFutures = new ArrayList<ScheduledFuture<?>>(1);
6059
private LifeCycleManager lifeCycleManager;
6160
private SequenceNumberGenerator sequenceNumberGenerator;
@@ -166,9 +165,9 @@ public void start() {
166165
}
167166

168167
public void stop() {
169-
// We don't check "started" here, because the executor service uses
168+
// We don't check "started" here, because the executor services use
170169
// lazy initialization, rather than being created in the start method
171-
stopExecutorService();
170+
stopExecutorServices();
172171

173172
started = false;
174173
}
@@ -216,14 +215,17 @@ public Object getConfigurationLock() {
216215
return configurationLock;
217216
}
218217

218+
219+
219220
@Override
220-
/**
221-
* @deprecated replaced by getScheduledExecutorService
222-
*/
223221
public synchronized ExecutorService getExecutorService() {
224-
return getScheduledExecutorService();
222+
if (threadPoolExecutor == null) {
223+
threadPoolExecutor = (ThreadPoolExecutor) ExecutorServiceUtil.newThreadPoolExecutor();
224+
}
225+
return threadPoolExecutor;
225226
}
226227

228+
227229
@Override
228230
public synchronized ScheduledExecutorService getScheduledExecutorService() {
229231
if (scheduledExecutorService == null) {
@@ -232,11 +234,12 @@ public synchronized ScheduledExecutorService getScheduledExecutorService() {
232234
return scheduledExecutorService;
233235
}
234236

235-
private synchronized void stopExecutorService() {
236-
if (scheduledExecutorService != null) {
237-
ExecutorServiceUtil.shutdown(scheduledExecutorService);
238-
scheduledExecutorService = null;
239-
}
237+
private synchronized void stopExecutorServices() {
238+
ExecutorServiceUtil.shutdown(scheduledExecutorService);
239+
scheduledExecutorService = null;
240+
241+
ExecutorServiceUtil.shutdown(threadPoolExecutor);
242+
threadPoolExecutor = null;
240243
}
241244

242245
private void removeShutdownHook() {

‎logback-core/src/main/java/ch/qos/logback/core/CoreConstants.java‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,12 @@ public class CoreConstants {
2525
public static final int CORE_POOL_SIZE = 0;
2626

2727
// Apparently ScheduledThreadPoolExecutor has limitation where a task cannot be
28-
// submitted from
29-
// within a running task unless the pool has worker threads already available.
30-
// ThreadPoolExecutor
31-
// does not have this limitation.
32-
// This causes tests failures in
33-
// SocketReceiverTest.testDispatchEventForEnabledLevel and
28+
// submitted from within a running task unless the pool has worker threads already available.
29+
// ThreadPoolExecutor does not have this limitation.
30+
// This causes tests failures in SocketReceiverTest.testDispatchEventForEnabledLevel and
3431
// ServerSocketReceiverFunctionalTest.testLogEventFromClient.
3532
// We thus set a pool size > 0 for tests to pass.
36-
public static final int SCHEDULED_EXECUTOR_POOL_SIZE = 1;
33+
public static final int SCHEDULED_EXECUTOR_POOL_SIZE = 2;
3734

3835
/**
3936
* Maximum number of threads to allow in a context's executor service.

‎logback-core/src/main/java/ch/qos/logback/core/net/AbstractSocketAppender.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ public void start() {
145145
deque = queueFactory.newLinkedBlockingDeque(queueSize);
146146
peerId = "remote peer " + remoteHost + ":" + port + ": ";
147147
connector = createConnector(address, port, 0, reconnectionDelay.getMilliseconds());
148-
task = getContext().getScheduledExecutorService().submit(new Runnable() {
148+
task = getContext().getExecutorService().submit(new Runnable() {
149149
@Override
150150
public void run() {
151151
connectSocketAndDispatchEvents();

0 commit comments

Comments
 (0)