[ 
https://issues.apache.org/jira/browse/JAMES-2542?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16605479#comment-16605479
 ] 

Tellier Benoit commented on JAMES-2542:
---------------------------------------

Hi!

Thanks for the report.

I had a quick look.

It turns out the code is not attempting to lock the mail if it was locked  by 
another concurent thread. The wasLocked variable has to be understood as "was 
locked by another thread" and thus negates the "was locked by this thread"...

I agree with the fact that naming is VERY misleading.

I must admit I do not understand the way this class works (and how such a 
locking logic protect us...)

Before changing the logic of the code here, we should write tests that 
demonstrate the broken behaviour so that we can, later on, fix it...

Cheers,

Benoit Tellier



> AbstractMailRepository locking/unlocking issue when storing
> -----------------------------------------------------------
>
>                 Key: JAMES-2542
>                 URL: https://issues.apache.org/jira/browse/JAMES-2542
>             Project: James Server
>          Issue Type: Bug
>          Components: MailStore & MailRepository
>    Affects Versions: master
>            Reporter: Abdou Ousmane Issoufou
>            Priority: Critical
>
> In AbstractMailRepository.java#store(Mail mc) there is an issue in the 
> finally block because the condition of the if statement is !waslocked instead 
> of waslocked. The comment also needs to be updated to "If it was locked, we 
> need to unlock now"
>  
> {code:java}
> // AbstractMailRepository.java
> @Override
> public MailKey store(Mail mc) throws MessagingException {
> boolean wasLocked = true;
> MailKey key = MailKey.forMail(mc);
> try {
> synchronized (this) {
> wasLocked = lock.isLocked(key);
> if (!wasLocked) {
> // If it wasn't locked, we want a lock during the store
> lock(key);
> }
> }
> internalStore(mc);
> return key;
> } catch (MessagingException e) {
> LOGGER.error("Exception caught while storing mail {}", key, e);
> throw e;
> } catch (Exception e) {
> LOGGER.error("Exception caught while storing mail {}", key, e);
> throw new MessagingException("Exception caught while storing mail " + key, e);
> } finally {
> if (!wasLocked) {
> // If it wasn't locked, we need to unlock now
> unlock(key);
> synchronized (this) {
> notify();
> }
> }
> }
> }
> {code}
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org

Reply via email to