Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix thread synchronization issues #43

Closed
camann9 opened this issue Dec 11, 2014 · 3 comments
Closed

Fix thread synchronization issues #43

camann9 opened this issue Dec 11, 2014 · 3 comments
Assignees
@marcelmay
Labels
bug
Milestone
1.5

Comments

@camann9
Copy link
Contributor

camann9 commented Dec 11, 2014

There are various issues in thread synchronization that are uncovered by the MultiRequestTest (execute multiple times and look at command line output). Those should be fixed. While doing this it might be useful to do a general refactoring of the protocol code.
@camann9 camann9 added the bug label Dec 11, 2014
@camann9 camann9 added this to the 1.5 milestone Dec 11, 2014
@buildscientist buildscientist mentioned this issue Dec 15, 2014
Closed
@buildscientist
Copy link
Member

I am unable to reproduce this - did you run the test multiple times in separate JVM processes? Maybe I can use JMeter to generate enough load to force this issue to happen.

@camann9
Copy link
Contributor Author

camann9 commented Dec 15, 2014

No, just run test20Senders20x4RetrieversAtTheSameTime from your IDE. Maybe increase the number of threads by increasing num.
I just tried with num=50 and found an exception:

5010    WARN           imap.ImapHandler| Can not close output stream
java.lang.NullPointerException
at com.icegreen.greenmail.imap.ImapHandler.close(ImapHandler.java:142)
at com.icegreen.greenmail.imap.ImapHandler.run(ImapHandler.java:100)
at com.icegreen.greenmail.server.AbstractServer$1.run(AbstractServer.java:101)
at java.lang.Thread.run(Thread.java:744)

Probably there are more of them, though I already fixed some.

@marcelmay marcelmay self-assigned this May 10, 2015
@marcelmay
Copy link
Member

Sonar reports 2 blocker issues "Methods "wait(...)", "notify()" and "notifyAll()" should never be called on Thread instances" for AbstractServer and Service. These issues should also be included.

marcelmay added a commit that referenced this issue May 13, 2015
@marcelmay
Issue #43 : Working on threading issue and server cleanup
8847099 
- Refactored Service into interface, as AbstractServer is the base implmentation.
- Cleaned up Service methods (unused params etc.)
- Switched startup loop-sleep-wait-with-timeout into notify-wait and simplified GreenMail#start()
marcelmay added a commit that referenced this issue May 13, 2015
@marcelmay
Issue #43 : Working on threading issue and server cleanup
909f64c 
- Removed obsolete waiting loop when opening server socket
marcelmay added a commit that referenced this issue May 13, 2015
@marcelmay
Issue #43 : Working on threading issue and server cleanup
25da3d0 
- Cleanup of AbstractServer.run() : Removed obsolete notifyAll
marcelmay added a commit that referenced this issue May 13, 2015
@marcelmay
Issue #43 : Working on threading issue and server cleanup
d7535b3 
- Make sure notifyAll is in synchronized block
marcelmay added a commit that referenced this issue May 13, 2015
@marcelmay
Issue #43 : Working on threading issue and server cleanup
f1f612d 
- AbstactServer: First terminate server thread THEN the client threads
marcelmay added a commit that referenced this issue May 13, 2015
@marcelmay
Issue #43 : Working on threading issue and server cleanup
febcaa9 
- Clear handlers when shutting down
- Handle spurious wakeups in #waitTillRunning()
marcelmay added a commit that referenced this issue May 13, 2015
@marcelmay
Issue #43 : Working on threading issue and server cleanup
78cd995 
- Fixed ImapRequestLineReader ignoring EOF, causing potential thread hang/leak
  (eg. when running ImapServerTest.testRetreiveSimpleWithNonDefaultPassword() ):

- ImapHandler cleanup (dead code, buffering, resource closing and handling socket/input stream close issue)
marcelmay added a commit that referenced this issue May 13, 2015
@marcelmay
Issue #43 : Working on threading issue and server cleanup
cff5e7c 
- Fixed profiling hotspot by caching immutable hashCode computation
marcelmay added a commit that referenced this issue May 13, 2015
@marcelmay
Issue #43 : Working on threading issue and server cleanup
c2ba580 
- Always close connected store (GreenMailUtil.get/setQuota)
- Sonar fixes
marcelmay added a commit that referenced this issue May 30, 2015
@marcelmay
Fix thread synchronization issues (issue #43)
e81bba7 
When closing, there can be a race between Handler thread and external closing of the connection
marcelmay added a commit that referenced this issue Jun 14, 2015
@marcelmay
Issue #43: Fix threading issue
3e0d411 
- Fixed server startup/shutdown race. This can cause still running services when quickly starting/stopping GreenMail.
marcelmay added a commit to marcelmay/greenmail that referenced this issue Jul 26, 2015
@marcelmay
Issue greenmail-mail-test#43 : Working on threading issue and server
2008906 
Try to avoid startup wait timeouts by always notifying waiting threads
marcelmay added a commit that referenced this issue Sep 17, 2015
@marcelmay
Issue #43: Fix threading issue
b003a55 
Always close server socket in run loop.

Previously, GreenMail only closed server socket when stopping the server.
This sometimes did not work when repeatedly starting/stopping the server using Circle CI (see Issue #69).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@marcelmay marcelmay

Labels
bug
Projects
None yet
Milestone
1.5
Development

No branches or pull requests
3 participants
@marcelmay @buildscientist @camann9