james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tellier Benoit (JIRA)" <server-...@james.apache.org>
Subject [jira] [Commented] (JAMES-2542) AbstractMailRepository locking/unlocking issue when storing
Date Thu, 06 Sep 2018 08:47:00 GMT

    [ 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 &amp; 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-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message