[jira] [Commented] (JAMES-3477) MimeMessageCopyOnWriteProxy is not thread safe
[ https://issues.apache.org/jira/browse/JAMES-3477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17255823#comment-17255823 ] ASF GitHub Bot commented on JAMES-3477: --- chibenwa commented on pull request #280: URL: https://github.com/apache/james-project/pull/280#issuecomment-751957047 I started a mailig list discussion https://www.mail-archive.com/server-dev@james.apache.org/msg69361.html This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > MimeMessageCopyOnWriteProxy is not thread safe > -- > > Key: JAMES-3477 > URL: https://issues.apache.org/jira/browse/JAMES-3477 > Project: James Server > Issue Type: Wish >Reporter: Benoit Tellier >Priority: Major > > https://www.mail-archive.com/server-dev@james.apache.org/msg69221.html > & > https://github.com/jeantil/james-project/commit/c0354ea21c5b0a8f6d46e9919f7db0d97db9eb23 > proves there is a concurrency issue in MimeMessageCopyOnWriteProxy class that > we need to investigate. > It causes our test suite to be flacky. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
[jira] [Commented] (JAMES-3477) MimeMessageCopyOnWriteProxy is not thread safe
[ https://issues.apache.org/jira/browse/JAMES-3477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17255821#comment-17255821 ] ASF GitHub Bot commented on JAMES-3477: --- chibenwa commented on pull request #282: URL: https://github.com/apache/james-project/pull/282#issuecomment-751957067 I started a mailig list discussion https://www.mail-archive.com/server-dev@james.apache.org/msg69361.html This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > MimeMessageCopyOnWriteProxy is not thread safe > -- > > Key: JAMES-3477 > URL: https://issues.apache.org/jira/browse/JAMES-3477 > Project: James Server > Issue Type: Wish >Reporter: Benoit Tellier >Priority: Major > > https://www.mail-archive.com/server-dev@james.apache.org/msg69221.html > & > https://github.com/jeantil/james-project/commit/c0354ea21c5b0a8f6d46e9919f7db0d97db9eb23 > proves there is a concurrency issue in MimeMessageCopyOnWriteProxy class that > we need to investigate. > It causes our test suite to be flacky. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
[jira] [Commented] (JAMES-3477) MimeMessageCopyOnWriteProxy is not thread safe
[ https://issues.apache.org/jira/browse/JAMES-3477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17255822#comment-17255822 ] Benoit Tellier commented on JAMES-3477: --- I started a mailig list discussion https://www.mail-archive.com/server-dev@james.apache.org/msg69361.html > MimeMessageCopyOnWriteProxy is not thread safe > -- > > Key: JAMES-3477 > URL: https://issues.apache.org/jira/browse/JAMES-3477 > Project: James Server > Issue Type: Wish >Reporter: Benoit Tellier >Priority: Major > > https://www.mail-archive.com/server-dev@james.apache.org/msg69221.html > & > https://github.com/jeantil/james-project/commit/c0354ea21c5b0a8f6d46e9919f7db0d97db9eb23 > proves there is a concurrency issue in MimeMessageCopyOnWriteProxy class that > we need to investigate. > It causes our test suite to be flacky. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
The case of javax.mail MimeMessage CopyOnWrite optimization
Hello there! We had been discussing on GitHub recently about an optimization in james core around the usage of MimeMessage. Javax.mail MimeMessage is currently used to represent a message of an email as part of the mail processing in James. It is part of the Mail interface (mailet-api). As Mail envelope is composed of several recipients, mail related operations are performed once for all these recipients (we enqueue the mail one time, we strip bcc one time etc...). Troubles arise when we need different behaviors as part of mail processing across recipients (think remote recipients, that needs there mail to be relayed, versus local recipients that needs to be locally delivered). The email get's duplicated (in MatcherSplitter) and the processing will then be distinct for both entities. The underlying MimeMessage may - or may not be modified. In order to prevent MimeMessage duplication in the event the underlying MimeMessage is not modified, a Copy On Write mechanism was introduced (I guess... Sorry, I was not there yet). Upon his CI effort, Jean Helou with the help of Matthieu Baechler made he unpleasant finding that this was not thread safe, that was leading to build instabilities. The mailet processing happens in Camel, which is multi-threaded. Concurrency issues arised between modifications, and message disposal, when a same MimeMessage instance was shared. [1] A first effort was to try to achieve thread-safety, which leaded to a brittle double reetrant read-write locks in order to govern data access. However, another performance enhancement bypassed these lock mechanism (MimeMessageWrapper allows accessing the data as an InputStream instead of requiring to copy it). The effort seemed overwhelming, not to metion possible risks of dead-locks. [2] We then came up with an always copy implementation [3]. Simpler, safer... The underlying logic is to avoid trying being smarter than mutability, and leverage immutability to achieve thread safety, which is a classic functional programming idiom. JMH benchmarks were conducted. We highlighter little performance difference for small messages, in the percent realm for both memory allocation and compute time. Differences are however higher for bigger messages (~10%) for both metrics. Please note that above 100KB the MimeMessage would be stored on disk, thus limiting memory impact (see MimeMessageInputStreamSource). Maybe we should make the threshold configurable, via a system property for instance? I just want to further mentioned I encountered that very issue on a production instance: the underlying email had been corrupted by the above mentioned COW bug and kept throwing NullPointerExceptions every time the content was accessed. This resulted (on top of distributed-james) in a RabbitMQ nack of the message, that ended up in a dead-letter queue. Replaying its processing required admin intervention and had been interpreted by the user as an email loss... To conclude this effort we (Jean an I) would like to merge the "Always copy" pull request. Also, would it be beneficial to write an ADR about this topic? Thoughts? Cheers, Benoit [1] The unfamous COW bug: https://github.com/apache/james-project/pull/280/commits/09b5554bbcbbb98757910d59bac54f97ee1f8b4f [2] The double nested reetrant read-write lock fix attempt: https://github.com/apache/james-project/pull/280 [3] The always copy fix: https://github.com/apache/james-project/pull/282 [4] Benchmarks: https://github.com/apache/james-project/pull/280#issuecomment-745211736 & https://github.com/apache/james-project/pull/280#issuecomment-745701937 [5] The JIRA ticket: https://issues.apache.org/jira/browse/JAMES-3477 - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
Re: Jenkins CI setup
Hello Jean, Nice work! Le 28/12/2020 à 23:21, Jean Helou a écrit : > Hello again jamers! > > It's time for a new irregular report on the CI effort on apache infra 🎅 ! > > Let's start with the good news : today I finally reached a successful build > https://builds.apache.org/blue/organizations/jenkins/james%2FApacheJames/detail/PR-268/45/pipeline > (the first fully successful build on apache infra) \o/ > > You can see in the pipeline that as discussed before the testing phase is > split in 2 parts: stable tests vs unstable tests, failure in the first > phase will fail the build, failures in the unstable phase will not be > considered a build failure (but should still collect the failed tests in > the reports, however the recent failures where mostly memory related in > which case the surefire report is not generated :( ) > > Over the last 2 weeks and 45 build attempts, I tagged all failing tests as > "Unstable", I also increased the heap in the forked surefire to resolve > some of the OutOfMemoryException failures > > At this stage I would really like to see this merged (if only to be able to > evaluate dangerous changes such as > https://github.com/apache/james-project/pull/282) +1 I think this however deserves a separate thread. I will start it now. > > You can look at https://github.com/apache/james-project/pull/268 to see > which tests have been marked as Unstable. It was rebased on master this > morning and I intend to clean up the history tonight. Will do, maybe tomorrow. On the principles, it is a yes from my side. As someone operating another CI, I want to play even unstable test on every runs. Is there some adaptation needed to do this? > I also removed some invasive logging from the webadmin test code (it used > to log every single http request made in the tests) the full log is still a > bit over 30MB... Nice enhancement, it is likely some long-forgotten debug statements. > > Best regards, > Jean > > On Fri, Dec 11, 2020 at 12:25 PM Jean Helou wrote: > >> I conclude that my effort to get CI working is cursed by the gods, >> remember : >> {"message":"No such image: quay.io/testcontainers/ryuk:0.2.3"} >>> >>> which repeats for most tests failures, this seems to be common enough >>> that there is stack overflow for it >>> >>> https://stackoverflow.com/questions/61887363/testcontainers-cant-pull-ryuk-image-quay-io-is-not-reachable >>> I have attempted to upgrade test containers to 1.15.0 (as it will pull >>> ryuk from docker hub instead of quay.io since 1.14.3 and we were using >>> 1.12) >>> hopefully this will help :) >>> >> >> A docker API change broke most of testcontainers versions, which won't be >> able to pull the images if they are not already available locally ! >> https://github.com/testcontainers/testcontainers-java/issues/3574 >>> yes, this Docker API change applies to most of Testcontainers versions. >> >> They should release a 1.15.1 to resolve the issue shortly, I have tried >> explicitly pulling the image in the steps of running the tests but sadly it >> doesn't seem to have helped :( >> >> jean >> >>> > - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
Re: JMAP vacation mailet
Hello Jean, Sorry for the late answer. Le 27/12/2020 à 01:29, Jean Helou a écrit : > Hi, > > While playing around with assemblies, I got an error that vacation mailet > configuration was missing when I added the JMAP module > https://github.com/apache/james-project/blob/master/server/container/guice/protocols/jmap/src/main/java/org/apache/james/jmap/draft/JMAPModule.java#L122 This check was added to ensure the mailetcontainer was correctly configured to answer people upon vacations. The JMAP module will require this check, as far as I remember even if the "JMAP protocol" itself is disabled (likely these checks should not be performed if the configuration disables the JMAP protocol?) On the subject of those checks, I personally do not like them yet... It's a constraint but they are easy to cheat, the security they provide is limited, but it restrict what as an administrator I could do... Like re-organizing mailetcontainer.xml to perform a single call to RecipientIsLocal (and thus to the LDAP) which has a performance and resiliency impact. > > I'm not very familiar with jmap and checked a draft at > https://tools.ietf.org/html/draft-ietf-jmap-mail-16#section-1.3 > As far as I understand the vacation mailet it is an additional capability > and could thus be made optional, am I correct ? You are right. Now, does it makes sense to expose the JMAP vacation extension in a separate Guice modules? Is there some cases we want JMAP but not the vacations? By the way allowing people to write their own custom JMAP extensions might become soon a topic, hopefully ;-) > > thanks > jean > Cheers, Benoit - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
[jira] [Commented] (JAMES-3431) Relay DSN options on RemoteDelivery
[ https://issues.apache.org/jira/browse/JAMES-3431?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17255809#comment-17255809 ] Benoit Tellier commented on JAMES-3431: --- https://github.com/linagora/james-project/pull/4181 carries over DSN options to remote SMTP servers relying on javax.mail SMTP server. Integration tests had been written using the Mock SMTP server previously modified. Please note that the current work suffer from the following flows, that are limitations induces by the use of javax.mail: - No ORCPT support. This RCPT DSN parameter will be omitted on exchanges with remote SMTP servers. The impact is likely low: the value matches the RCPT TO, even in the case of mailing list rewrites. Maybe it is of some uses in the advent of aliases, for the sender MUA to display the actual user this mail was sent to. - As NOTIFY parameter is set on a per-SMTPMessage basis, we have to transmit several time the message (one time for each combinasons of NOTIFY parameters used for recipients of the message - recipients with the same notify parameters can be grouped together). The impact is low: it incurs a small performance cost. Also, I would be surprised MUAs set per-recipients values on sent emails and not global values. We now remain to write the integration tests listed above. > Relay DSN options on RemoteDelivery > --- > > Key: JAMES-3431 > URL: https://issues.apache.org/jira/browse/JAMES-3431 > Project: James Server > Issue Type: Bug > Components: Remote Delivery, SMTPServer >Affects Versions: 3.5.0 >Reporter: Karsten Otto >Priority: Major > > Since James claims to support the DSN SMTP extension, it may receive a mail > submission according to [RFC 3461|https://tools.ietf.org/html/rfc3461]: > {code:java} > MAIL FROM: RET=HDRS ENVID=QQ314159 > RCPT TO: NOTIFY=SUCCESS,FAILURE,DELAY > ORCPT=rfc822;d...@ivory.edu > RCPT TO: NOTIFY=NEVER{code} > In this case James should > * remember the given DSN options (NOTIFY, ORCPT, RET, ENVID) for each > recipient, and > * provide the same options when relaying the mail to remote servers via the > RemoteDelivery mailet. > (The DSN options should be accessible to other interested mailets as well, > e.g. for bounce processing.) > Possibly related issues: > https://issues.apache.org/jira/browse/JAMES-322 > https://issues.apache.org/jira/browse/JAMES-362 > -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
[jira] [Created] (JAMES-3486) Adapt MailboxChangesMethodContract for stability against distributed environment
Lan Khuat created JAMES-3486: Summary: Adapt MailboxChangesMethodContract for stability against distributed environment Key: JAMES-3486 URL: https://issues.apache.org/jira/browse/JAMES-3486 Project: James Server Issue Type: Sub-task Reporter: Lan Khuat h3. Objective Because changes in distributed environment do not happen instantaneously, we need to adapt the contract so that the tests behave in a more reliable way. h3. How Before, we were storing a state manually as a reference point, then the change(s) that we interested in would be conducted after that. This will not work in distributed environment, since the reference state might be stored even before the provisioning process complete and leads to unpredictable result. * We will wait for a new state to be recorded successfully each time there is a change happen * Fetch them sequentially until all the preparation steps are completed * Mark the latest stage * Conduct the change that we are interested in * fetch the result with the latest state as reference point. h3. DoD Integration tests for MailboxChangesMethod should run reliably in distributed environment. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
[jira] [Commented] (JAMES-3477) MimeMessageCopyOnWriteProxy is not thread safe
[ https://issues.apache.org/jira/browse/JAMES-3477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17255778#comment-17255778 ] ASF GitHub Bot commented on JAMES-3477: --- chibenwa commented on a change in pull request #282: URL: https://github.com/apache/james-project/pull/282#discussion_r549547128 ## File path: server/container/core/src/main/java/org/apache/james/server/core/MimeMessageInputStream.java ## @@ -40,19 +40,13 @@ *the message to wrap * @param tryCast *try to cast the {@link MimeMessage} to - *{@link MimeMessageCopyOnWriteProxy} / *{@link MimeMessageWrapper} to do some optimized processing if *possible * @throws MessagingException */ public MimeMessageInputStream(MimeMessage message, boolean tryCast) throws MessagingException { MimeMessage m = message; -// check if we need to use the wrapped message -if (tryCast && m instanceof MimeMessageCopyOnWriteProxy) { -m = ((MimeMessageCopyOnWriteProxy) m).getWrappedMessage(); -} - // check if we can use optimized operations if (tryCast && m instanceof MimeMessageWrapper) { Review comment: Agree and already applied ;-) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > MimeMessageCopyOnWriteProxy is not thread safe > -- > > Key: JAMES-3477 > URL: https://issues.apache.org/jira/browse/JAMES-3477 > Project: James Server > Issue Type: Wish >Reporter: Benoit Tellier >Priority: Major > > https://www.mail-archive.com/server-dev@james.apache.org/msg69221.html > & > https://github.com/jeantil/james-project/commit/c0354ea21c5b0a8f6d46e9919f7db0d97db9eb23 > proves there is a concurrency issue in MimeMessageCopyOnWriteProxy class that > we need to investigate. > It causes our test suite to be flacky. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
[jira] [Commented] (JAMES-3446) Outlook Mobile App Imap request error
[ https://issues.apache.org/jira/browse/JAMES-3446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17255755#comment-17255755 ] seungmin commented on JAMES-3446: - As a result, is there no way to solve the Outlook mobile problem? > Outlook Mobile App Imap request error > -- > > Key: JAMES-3446 > URL: https://issues.apache.org/jira/browse/JAMES-3446 > Project: James Server > Issue Type: Bug >Affects Versions: 3.0-beta4 >Reporter: seungmin >Priority: Major > Fix For: 3.0.0 > > Attachments: 20201117.cap, 20201117.cap, Capture d’écran de > 2020-11-17 09-57-45.png, image-2020-11-14-16-17-51-011.png, > image-2020-11-16-10-31-15-497.png, image-2020-11-16-14-23-24-498.png, > image-2020-11-16-14-25-06-005.png, image-2020-11-16-16-30-07-096.png, > image-2020-11-16-16-33-36-050.png, image-2020-11-16-16-33-37-781.png, > image-2020-11-16-16-58-33-360.png, image-2020-11-16-17-22-57-210.png, > image-2020-11-17-00-26-57-359.png, image-2020-11-17-00-27-10-493.png, > image-2020-11-17-00-27-58-590.png, image-2020-11-17-00-29-16-919.png, > image-2020-11-17-00-30-23-988.png, image-2020-11-17-00-31-42-025.png, > image-2020-11-18-16-09-15-340.png, image-2020-11-18-16-57-57-404.png, > image-2020-11-18-16-58-07-743.png, image-2020-11-18-18-08-55-845.png, > image-2020-11-19-16-51-07-279.png, image-2020-11-20-22-17-45-426.png, > image-2020-11-20-23-32-56-694.png, image-2020-11-21-02-45-12-876.png, > image-2020-11-30-00-48-06-447.png, image-2020-11-30-00-49-14-281.png, > image-2020-11-30-00-49-17-944.png, image-2020-11-30-00-55-59-761.png, > image-2020-11-30-00-56-26-139.png, image-2020-12-04-00-25-29-953.png > > > !image-2020-11-14-16-17-51-011.png! > > - only outlook mobile app > Error while processing imap request > cosumeWord source same master james source > but gmail app, samsung mail app no problem > > "only" outlook mobile issue -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
Re: Jenkins CI setup
Hello again jamers! It's time for a new irregular report on the CI effort on apache infra 🎅 ! Let's start with the good news : today I finally reached a successful build https://builds.apache.org/blue/organizations/jenkins/james%2FApacheJames/detail/PR-268/45/pipeline (the first fully successful build on apache infra) You can see in the pipeline that as discussed before the testing phase is split in 2 parts: stable tests vs unstable tests, failure in the first phase will fail the build, failures in the unstable phase will not be considered a build failure (but should still collect the failed tests in the reports, however the recent failures where mostly memory related in which case the surefire report is not generated :( ) Over the last 2 weeks and 45 build attempts, I tagged all failing tests as "Unstable", I also increased the heap in the forked surefire to resolve some of the OutOfMemoryException failures At this stage I would really like to see this merged (if only to be able to evaluate dangerous changes such as https://github.com/apache/james-project/pull/282), I will make a You can look at https://github.com/apache/james-project/pull/268 to see which tests have been marked as Unstable. It was rebased on master this morning and I intend to clean up the history tonight. I also removed some invasive logging from the webadmin test code (it used to log every single http request made in the tests) the full log is still a bit over 30MB... Best regards, Jean On Fri, Dec 11, 2020 at 12:25 PM Jean Helou wrote: > I conclude that my effort to get CI working is cursed by the gods, > remember : > >> > {"message":"No such image: quay.io/testcontainers/ryuk:0.2.3"} >> >> which repeats for most tests failures, this seems to be common enough >> that there is stack overflow for it >> >> https://stackoverflow.com/questions/61887363/testcontainers-cant-pull-ryuk-image-quay-io-is-not-reachable >> I have attempted to upgrade test containers to 1.15.0 (as it will pull >> ryuk from docker hub instead of quay.io since 1.14.3 and we were using >> 1.12) >> hopefully this will help :) >> > > A docker API change broke most of testcontainers versions, which won't be > able to pull the images if they are not already available locally ! > https://github.com/testcontainers/testcontainers-java/issues/3574 > > yes, this Docker API change applies to most of Testcontainers versions. > > They should release a 1.15.1 to resolve the issue shortly, I have tried > explicitly pulling the image in the steps of running the tests but sadly it > doesn't seem to have helped :( > > jean > >>
[jira] [Commented] (JAMES-3477) MimeMessageCopyOnWriteProxy is not thread safe
[ https://issues.apache.org/jira/browse/JAMES-3477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=1727#comment-1727 ] ASF GitHub Bot commented on JAMES-3477: --- jeantil commented on a change in pull request #282: URL: https://github.com/apache/james-project/pull/282#discussion_r549327693 ## File path: server/container/core/src/main/java/org/apache/james/server/core/MimeMessageInputStream.java ## @@ -40,19 +40,13 @@ *the message to wrap * @param tryCast *try to cast the {@link MimeMessage} to - *{@link MimeMessageCopyOnWriteProxy} / *{@link MimeMessageWrapper} to do some optimized processing if *possible * @throws MessagingException */ public MimeMessageInputStream(MimeMessage message, boolean tryCast) throws MessagingException { MimeMessage m = message; -// check if we need to use the wrapped message -if (tryCast && m instanceof MimeMessageCopyOnWriteProxy) { -m = ((MimeMessageCopyOnWriteProxy) m).getWrappedMessage(); -} - // check if we can use optimized operations if (tryCast && m instanceof MimeMessageWrapper) { Review comment: you definitely want to remove all the synchronized blocks on MimeMessageWrapper since it is definitely not thread safe (f only because it relies on MimeMessageInputStreamSource) which itself isn't thread safe. This will both remove the illusion of thread safety, provide performance improvements and possibly let the integration test fail faster if there are concurrent access left. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > MimeMessageCopyOnWriteProxy is not thread safe > -- > > Key: JAMES-3477 > URL: https://issues.apache.org/jira/browse/JAMES-3477 > Project: James Server > Issue Type: Wish >Reporter: Benoit Tellier >Priority: Major > > https://www.mail-archive.com/server-dev@james.apache.org/msg69221.html > & > https://github.com/jeantil/james-project/commit/c0354ea21c5b0a8f6d46e9919f7db0d97db9eb23 > proves there is a concurrency issue in MimeMessageCopyOnWriteProxy class that > we need to investigate. > It causes our test suite to be flacky. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
[jira] [Commented] (JAMES-3477) MimeMessageCopyOnWriteProxy is not thread safe
[ https://issues.apache.org/jira/browse/JAMES-3477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17255517#comment-17255517 ] ASF GitHub Bot commented on JAMES-3477: --- chibenwa commented on pull request #282: URL: https://github.com/apache/james-project/pull/282#issuecomment-751667026 `If you are certain that concurrent access may no longer occur on MimeMessageInputStreamSource` ... How can I be sure of such a thing? :-) ` I suggest dropping the misleading synchronized.` +1 this will make clear it is not thread safe... Just like we did in MimeMessageWrapper. `on a side note: List streams the ordering property of lists doesn't seem to have any importance in the implementation, a Set would be more appropriate.` true `I guess we will have to wait and see if shared access to MimeMessageWrapper instances occurs or not.` I pretty much agree with this statement... This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > MimeMessageCopyOnWriteProxy is not thread safe > -- > > Key: JAMES-3477 > URL: https://issues.apache.org/jira/browse/JAMES-3477 > Project: James Server > Issue Type: Wish >Reporter: Benoit Tellier >Priority: Major > > https://www.mail-archive.com/server-dev@james.apache.org/msg69221.html > & > https://github.com/jeantil/james-project/commit/c0354ea21c5b0a8f6d46e9919f7db0d97db9eb23 > proves there is a concurrency issue in MimeMessageCopyOnWriteProxy class that > we need to investigate. > It causes our test suite to be flacky. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
[jira] [Commented] (JAMES-3477) MimeMessageCopyOnWriteProxy is not thread safe
[ https://issues.apache.org/jira/browse/JAMES-3477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17255467#comment-17255467 ] ASF GitHub Bot commented on JAMES-3477: --- jeantil commented on pull request #282: URL: https://github.com/apache/james-project/pull/282#issuecomment-751631891 > Both pipelines will work on different email copies now no? In the event that there are 2 different MimeMessageWrapper instances, are we certain that DeferredOutputStream will have finished copying the underlying file ? I was worried that the file storing the email data for t1 might be deleted before the file storing the data for t2 is completely written ending up in a MimeMessageWrapper instance in t2 that's backed by a corrupted file. I read through more code and it looks like DeferredOutputStream does make a defensive copy (I was misled by the `deferred` in the name) so 2 different instances would have 2 complete files as soon as instance creation returns. That leaves the `synchronized` flag on `MimeMessageInputStreamSource#getInputStream` which suggests concurrent access may occur there with the possible outcome I mentionned. If you are certain that concurrent access may no longer occur on `MimeMessageInputStreamSource` I suggest dropping the misleading `synchronized`. I assume that the synchronization was added to avoid/fix a `ConcurrentModificationException` on the `List streams`. The `synchronized` is actually a lie since a `ConcurrentModificationException` could still be triggererd by threads calling `getInputStream` and dispose concurrently on a side note: `List streams` the ordering property of lists doesn't seem to have any importance in the implementation, a `Set` would be more appropriate. I guess we will have to wait and see if shared access to MimeMessageWrapper instances occurs or not. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > MimeMessageCopyOnWriteProxy is not thread safe > -- > > Key: JAMES-3477 > URL: https://issues.apache.org/jira/browse/JAMES-3477 > Project: James Server > Issue Type: Wish >Reporter: Benoit Tellier >Priority: Major > > https://www.mail-archive.com/server-dev@james.apache.org/msg69221.html > & > https://github.com/jeantil/james-project/commit/c0354ea21c5b0a8f6d46e9919f7db0d97db9eb23 > proves there is a concurrency issue in MimeMessageCopyOnWriteProxy class that > we need to investigate. > It causes our test suite to be flacky. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org