Skip to content

Commit a1f621f

Browse files
committed
#204, #205: fixed bug where executor could be used after shutting down. Threads are now named. Removed redundant warning suppressions
1 parent 91b427f commit a1f621f

File tree

2 files changed

+40
-22
lines changed

2 files changed

+40
-22
lines changed

src/main/java/org/simplejavamail/mailer/internal/mailsender/MailSender.java

+10-22
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@
2525
import java.util.concurrent.ExecutorService;
2626
import java.util.concurrent.Executors;
2727
import java.util.concurrent.Phaser;
28-
import java.util.concurrent.ThreadFactory;
29-
import java.util.concurrent.atomic.AtomicInteger;
3028

3129
import static java.lang.String.format;
3230
import static org.simplejavamail.converter.EmailConverter.mimeMessageToEML;
@@ -72,7 +70,7 @@ public class MailSender {
7270
* Only set when {@link ProxyConfig} is provided with authentication details.
7371
*/
7472
@Nullable
75-
private AnonymousSocks5Server proxyServer = null;
73+
private AnonymousSocks5Server proxyServer;
7674

7775
/**
7876
* Allows us to manage how many thread we run at the same time using a thread pool.
@@ -194,21 +192,9 @@ the proxy bridge server (or connection pool in async mode) while a non-async ema
194192
smtpRequestsPhaser.register();
195193
if (async) {
196194
// start up thread pool if necessary
197-
if (executor == null) {
198-
executor = Executors.newFixedThreadPool(operationalConfig.getThreadPoolSize(), new ThreadFactory() {
199-
final AtomicInteger threadCounter = new AtomicInteger(0);
200-
@Override
201-
public Thread newThread(Runnable r) {
202-
Thread thread = new Thread(r, "Simple Java Mail async mail sender #" + threadCounter.getAndIncrement());
203-
if (!thread.isDaemon()) {
204-
thread.setDaemon(true);
205-
}
206-
if (thread.getPriority() != Thread.NORM_PRIORITY) {
207-
thread.setPriority(Thread.NORM_PRIORITY);
208-
}
209-
return thread;
210-
}
211-
});
195+
if (executor == null || executor.isShutdown()) {
196+
executor = Executors.newFixedThreadPool(operationalConfig.getThreadPoolSize(),
197+
new NamedThreadFactory("Simple Java Mail async mail sender"));
212198
}
213199
configureSessionWithTimeout(session, operationalConfig.getSessionTimeout());
214200
executor.execute(new Runnable() {
@@ -269,7 +255,6 @@ private void sendMailClosure(@Nonnull final Session session, @Nonnull final Emai
269255

270256
try {
271257
synchronized (this) {
272-
//noinspection ConstantConditions
273258
if (needsAuthenticatedProxy() && !proxyServer.isRunning()) {
274259
LOGGER.trace("starting proxy bridge");
275260
proxyServer.start();
@@ -319,19 +304,23 @@ private void configureBounceToAddress(final Session session, final Email email)
319304
}
320305

321306
/**
322-
* We need to keep a count of running threads in case a proxyserver is running
307+
* We need to keep a count of running threads in case a proxyserver is running or a connection pool needs to be shut down.
323308
*/
324309
private synchronized void checkShutDownRunningProcesses() {
325310
smtpRequestsPhaser.arriveAndDeregister();
326311
LOGGER.trace("SMTP request threads left: {}", smtpRequestsPhaser.getUnarrivedParties());
327312
// if this thread is the last one finishing
328313
if (smtpRequestsPhaser.getUnarrivedParties() == 0) {
329314
LOGGER.trace("all threads have finished processing");
330-
//noinspection ConstantConditions
331315
if (needsAuthenticatedProxy() && proxyServer.isRunning() && !proxyServer.isStopping()) {
332316
LOGGER.trace("stopping proxy bridge...");
333317
proxyServer.stop();
334318
}
319+
// shutdown the threadpool, or else the Mailer will keep any JVM alive forever
320+
// executor is only available in async mode
321+
if (executor != null) {
322+
executor.shutdown();
323+
}
335324
}
336325
}
337326

@@ -389,7 +378,6 @@ public synchronized void testConnection() {
389378
logSession(session, "connection test");
390379

391380
try (Transport transport = session.getTransport()) {
392-
//noinspection ConstantConditions
393381
if (needsAuthenticatedProxy() && !proxyServer.isRunning()) {
394382
LOGGER.trace("starting proxy bridge for testing connection");
395383
proxyServer.start();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package org.simplejavamail.mailer.internal.mailsender;
2+
3+
import javax.annotation.Nonnull;
4+
import java.util.concurrent.ThreadFactory;
5+
import java.util.concurrent.atomic.AtomicInteger;
6+
7+
import static java.lang.String.format;
8+
import static java.lang.Thread.currentThread;
9+
10+
class NamedThreadFactory implements ThreadFactory {
11+
private final AtomicInteger threadNumber = new AtomicInteger(1);
12+
private final ThreadGroup group;
13+
private final String threadName;
14+
15+
NamedThreadFactory(@Nonnull final String threadName) {
16+
SecurityManager s = System.getSecurityManager();
17+
group = (s != null) ? s.getThreadGroup() : currentThread().getThreadGroup();
18+
this.threadName = threadName;
19+
}
20+
21+
@Nonnull
22+
public Thread newThread(@Nonnull Runnable r) {
23+
Thread t = new Thread(group, r, format("%s %d", threadName, threadNumber.getAndIncrement()));
24+
if (t.isDaemon())
25+
t.setDaemon(false);
26+
if (t.getPriority() != Thread.NORM_PRIORITY)
27+
t.setPriority(Thread.NORM_PRIORITY);
28+
return t;
29+
}
30+
}

0 commit comments

Comments
 (0)