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

Benoit Tellier commented on JAMES-3791:
---------------------------------------

Hello

Quick answer for now as I am in vacations, but this is a quite critical topic 
that in my opinion needs to be adressed timely.

First thanks for the report, and identification of the issue!

Second, the viable long term fix is a complete rewrite of the remote delivery 
code. The use of javax mail made its time there. CF 
https://issues.apache.org/jira/browse/JAMES-964 . Options ranges from NIO based 
alternatives like https://github.com/HubSpot/NioSmtpClient or maybe using the 
Netty SMTP codec. I would be glad to enforce a strong distinction between 
gatewaying (where connexion can be reused) and routing (where this can not be 
assumed).

Third, one could mitigate this bug today by setting RemoteDelivery threads to 
1, which equates performance wise to option 1 you mentionned. 

Option 1 is obviously not desirable for performance reasons as this means that 
effectively one mail can be sent at a time thus fixing a major bottleneck to 
mail sending, especially for relaying use cases, where high throughtut can be 
expected.

Option 2 do not please me either. James messages do not extend SMTPMessages, 
convertion implies a data copy (performance hit). Today this price is only 
payed when DSNs extensions are involved.

Option 3 could be to use several sessions, thus preventing concurrent use of 
the same session. A thread local could be doing it, as this is accessed from a 
single, bounded thread pool (in MailDelivrerToHost). Another ideacould be to 
rely on an object pool, dimensionned to match the count of threads.

Personnally I would go for the object pool, using commons-pool.

This also proves we need some serious concurrency tests on remoteDelivery, that 
today are missing and would prevent regressions on this topic.

> Remote Delivery sometimes uses wrong MAIL FROM address
> ------------------------------------------------------
>
>                 Key: JAMES-3791
>                 URL: https://issues.apache.org/jira/browse/JAMES-3791
>             Project: James Server
>          Issue Type: Bug
>          Components: Remote Delivery
>    Affects Versions: master, 3.7.0
>            Reporter: Karsten Otto
>            Priority: Major
>
> *Observed Issue:*
> When delivering a mail to a remote server, James sometimes under load uses 
> the wrong envelope sender (MAIL FROM). This creates a wrong Return-Path at 
> the recipients end, which among other things causes problems with DSN replies.
> *Analysis:*
> I traced this to MailDelivrerToHost.tryDeliveryToHost(), which shows in the 
> debug log a pattern like:
>  # Attempting delivery of ... from al...@example.org
>  # Mail sent successfully to ... from b...@example.org
> I also notice that getPropertiesForMail() modifies the shared SMTPSession by 
> putting the envelope sender into its properties. So this is likely a race 
> condition.
> *Possible Fixes:*
> I am not sure what the best fix for this would be:
>  # Make tryDeliveryToHost() synchronized (ugly)
>  # Use SMTPMessage.setEnvelopeSender() instead of SMTPSession properties
> Fix 2 is nicer as it preserves concurrency, but it requires that 
> mail.getMessage() is always an SMTPMessage (extends MimeMessage). I do not 
> know the James code base well enough to be certain of that.
> Any thoughts?



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
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