search

voodoodyne / subethasmtp

SubEtha SMTP is a Java library for receiving SMTP mail
Other
344 stars 138 forks source link

Multi-threading Issue #34

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
SubEthaSMTP server implementation creates a thread (Session class) for each 
incoming mail message to read from the open socket.  Session instances are 
tracked by use of a shared ThreadGroup defined in the server instance.  The 
ThreadGroup is using a relatively unsafe method call to activeCount().

From Java's Javadoc for ThreadGroup.activeCount() method:
   "Returns an estimate of the number of active threads in this thread group. The result might not reflect concurrent activity, and might be affected by the presence of certain system threads.  Due to the inherently imprecise nature of the result, it is recommended that this method only be used for informational purposes."

The call to activeCount() is used to determine if too many connections to the 
server exist (based on defined property max connections).  The value returned 
from activeCount() is not reliable and does not work effectively to determine 
if too many connections exist.

This design also has major issues when combined with any sort of 
multi-threading implemented by users of SubEthaSMTP.  For example, a 
MessageHandler instance may pass processing of the inbound message data to a 
thread pool of worker threads, relieving the Session thread to handle the next 
incoming message.  Threads created in the worker pool will by default exist in 
the same ThreadGroup as the Session thread, causing the activeCount() method to 
report the worker thread as a connection thread.  This leaves the server in an 
invalid state.  If threads in the worker pool are not reclaimed after 
performing their work, then the current connection count in the SMTP server 
instance will remain invalid/inconsistent.

In order to test this issue, simple create a MessageHandler implementation 
which spawns a new Thread which continues to run in a loop and then check the 
number of current connections to the SMTP.

One suggested fix is to have the SMTP server manage it's own pool of current 
Session instances, rather than relying on ThreadGroup.  Each created Session 
instance should be registered in a pool, and removed from the pool when 
completed.

Original issue reported on code.google.com by michael....@gmail.com on 2 Nov 2010 at 11:51

GoogleCodeExporter commented 9 years ago 
Interesting, we never thought of the custom worker pool issue.

Original comment by lhori...@gmail.com on 2 Nov 2010 at 6:14

GoogleCodeExporter commented 9 years ago 
I have to confirm that this can really be an issue!

I'm working on a small application to intercept SMTP messages coming from 
Postfix (for content filtering purposes etc.) and everything worked fine 
(thanks to SubEthaSMTP) without any issues during my first tests with a Java 
standalone application.

Today I converted the project to an OSGi bundle because I want to use Apache 
Karaf as a runtime environment for my application. Now everything works fine 
when I deploy my bundle into Karaf. It's still fine when the first message 
arrives and gets processed but after that the SMTPServer component stays 
unusable.

I don't know any details of Karaf's thread handling internals and how exactly 
this could interfere with the thread management of SubEthaSMTP but obviously 
the bug is very reproducible, at least on my machine, and it seems to be 
related to the unreliable methods of ThreadGroup. Everytime I redeploy the OSGi 
bundle it works exactly once but when the second mail arrives I get the 
following stack trace:

Exception in thread "org.subethamail.smtp.server.SMTPServer" 
java.lang.IllegalThreadStateException
        at java.lang.ThreadGroup.addUnstarted(ThreadGroup.java:843)
        at java.lang.Thread.init(Thread.java:348)
        at java.lang.Thread.<init>(Thread.java:476)
        at org.subethamail.smtp.server.Session.<init>(Session.java:76)
        at org.subethamail.smtp.server.SMTPServer.run(SMTPServer.java:406)
        at java.lang.Thread.run(Thread.java:662)

I'm not sure how much work it would be, but are there any plans to fix this 
issue? In fact I'm very happy with SubEthaSMTP and how easy to use it is. But 
in this case it's unfortunately a show-stopper. It would be great to have a 
working SMTP server again - even with with OSGi and Karaf.

Thanks for any feedback!

Marco

Original comment by m.ehrent...@gmail.com on 3 Apr 2011 at 7:49

GoogleCodeExporter commented 9 years ago 
I think the latter seems to be a different issue. According to the ThreadGroup 
source and the googled example http://forums.java.net/node/660343 this 
exception occurs if the ThreadGroup is destroyed. SubEthaSMTP does not call 
Thread.destroy anywhere. Maybe it is called by the container. It is plausibe 
that the ThreadGroup is destroyed on redeploy, but I cannot imagine, however, 
why the first mail arrives correctly, and the second not. Moreover, SMTPServer 
does create a new ThreadGroup each time it is started. So it seems the same 
SMTPServer instance is used even after redeploy. Maybe stop / start is called 
instead of creating a new instance?

I will copy the code from the POP3 server of Mireka, which maintains its own 
session thread list, instead of relying on a ThreadGroup. This will solve the 
original issue. Maybe it will solve the second too.

Original comment by hontvari@flyordie.com on 4 Apr 2011 at 1:28

GoogleCodeExporter commented 9 years ago 
Thanks for your help! Actually I didn't yet have the time to study your code so 
I just supposed that my problem could be related to this issue. 

What I already did is to make sure that only one instance of SMTPServer is 
created and that it is only started once! The application is based on 
PicoContainer IoC container which is configured to assure that exactly one 
instance of each class is created and injected. Furthermore I added excessive 
logging statements and analyzed the application and Karaf log to be sure that 
there's only one SMTPServer which is started once. I even tried to synchronize 
start/stop methods in my code and added a guard variable to ensure that the 
embedded SMTPServer instance is only created and started once. Without any 
success... 

Interestingly enough it worked without problems before I tried to deploy it 
into Karaf. What I find puzzling too is that it works for exactly one message 
each time. This doesn't really look like a nondeterministic threading issue.

Anyway I'll give it another try with your patch. Hopefully this will fix this 
issue.

Original comment by m.ehrent...@gmail.com on 4 Apr 2011 at 6:44

GoogleCodeExporter commented 9 years ago 
Fixed in r414, ThreadGroup is no longer used to maintain the list of active 
sessions.

Original comment by hontvari@flyordie.com on 12 Apr 2011 at 6:44

GoogleCodeExporter commented 9 years ago 
Thank you so much, Levente!

I've tested my application with some long running realistic test runs and all 
kinds of emails from spam to mailing list mails etc. Everything works fine now 
- inside and outside of an OSGi container. Obviously this fix has solved my 
issue :)

Original comment by m.ehrent...@gmail.com on 17 Apr 2011 at 7:45

GoogleCodeExporter commented 9 years ago 
great :) Thanks for the verification.

Original comment by hontvari@flyordie.com on 17 Apr 2011 at 8:17