[
https://issues.apache.org/jira/browse/IMAP-372?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13631431#comment-13631431
]
Andrzej Rusin commented on IMAP-372:
------------------------------------
Hi Eric,
Take a look at this
(org.apache.james.mailbox.store.AbstractDelegatingMailboxListener.event(Event)):
public void event(Event event) {
MailboxPath path = event.getMailboxPath();
Map<MailboxPath, List<MailboxListener>> listeners = getListeners();
List<MailboxListener> mListeners = null;
synchronized (listeners) {
mListeners = listeners.get(path);
if (mListeners != null && mListeners.isEmpty() == false) {
// take snapshot of the listeners list for later
mListeners = new ArrayList<MailboxListener>(mListeners);
if (event instanceof MailboxDeletion) {
// remove listeners if the mailbox was deleted
listeners.remove(path);
} else if (event instanceof MailboxRenamed) {
// handle rename events
MailboxRenamed renamed = (MailboxRenamed) event;
List<MailboxListener> l = listeners.remove(path);
if (l != null) {
listeners.put(renamed.getNewPath(), l);
}
}
}
}
//outside the synchronized block against deadlocks from propagated
events wanting to lock the listeners
if (mListeners != null) {
int sz = mListeners.size();
for (int i = 0; i < sz; i++) {
MailboxListener l = mListeners.get(i);
l.event(event);
}
}
List<MailboxListener> globalListeners = getGlobalListeners();
if (globalListeners != null) {
synchronized (globalListeners) {
if (globalListeners.isEmpty() == false) {
List<MailboxListener> closedListener = new
ArrayList<MailboxListener>();
//TODO do not fire them inside synchronized block too?
int sz = globalListeners.size();
for (int i = 0; i < sz; i++) {
MailboxListener l = globalListeners.get(i);
l.event(event);
}
if (closedListener.isEmpty() == false) {
globalListeners.removeAll(closedListener);
}
}
}
}
}
In my opinion it does not risk ConcurrentModificationException, as:
- the Map is not touched outside synchronized block,
- to be safe I take a snapshot of the mListeners into a new List, that won't be
modified from outside.
But I may be wrong.
> Deadlock in AbstractDelegatingMailboxListener under load
> --------------------------------------------------------
>
> Key: IMAP-372
> URL: https://issues.apache.org/jira/browse/IMAP-372
> Project: James Imap
> Issue Type: Bug
> Components: Mailbox
> Reporter: Andrzej Rusin
> Assignee: Eric Charles
> Attachments: James Listener Deadlock.txt
>
>
> In AbstractDelegatingMailboxListener::event, firing the events inside the
> synchronized blocks causes a deadlock involving:
> lock on HashMapDelegatingMailboxListener::listeners
> (synchronized methods) lock on SelectedMailboxImpl
> In my example, these 3 methods got interlocked:
> AbstractDelegatingMailboxListener.addListener
> SelectedMailboxImpl.msn
> AbstractDelegatingMailboxListener.removeListener
> My idea to fix it is basically to take the for loop on mListeners outside the
> synchronized block in
> org.apache.james.mailbox.store.AbstractDelegatingMailboxListener.event(Event).
> That will not make the propagated event processing in SelectedMailboxImpl
> hit the synchronized lock on listeners.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
