Re: [jira] Commented: (JAMES-421) MailImpls sharing MimeMessage's!
> > IMHO the DEFAULT behaviour should be the deep copy. > > I disagree, what we're trying to do is to manage routing the message. > > Message can be routed down two different routes to its > destination, but is still the same message. -1 on that! James developers already did mistakes due to mimemessage shared and this bug report is the proof. Performance can be achieved with copyonwrite proxies or different approach but we should avoid that mailet writer run a mail.duplicate() and without the knowledge that it will share the mimemessage. NOONE will ever understand that running a matcher in a processor will duplicate the envelope but will share the mimemessage so if you change one of the 2 mails you don't know wether you will receive 2 identical messages or 2 different ones! > The default should be the least resource intensive. The default should be the SAFEST. The unsafe optimization should be applied understanding possible problems of the optimization. As I already said we can achieve optimization later or in a different way but first we should ensure that message handling is consistent and SAFE. > In fact I would tend to say that the issue is not technical, > but one of documentation. The docs should clearly explain > that and any changes should be made only to ensure that there > is a simple and consistent method which can be used to branch > the message itself. Please reread the bug report and not only the subject. The issue is REAL and is a critical BUG: if you think that MimeMessage sharing is good then it is a bug in the LinearProcessor. I don't care wether the message is shared or not, but the scenario I submitted should not give that result. The developer that wrote the LinearProcessor did not consider that the MailImpl.duplicate was returning a new Mail with the same MimeMessage. The developer that wrote the AbstractRedirect did the bugfree thing (clone the mimemessage). This is the "easy" code that should be documented to do the right thing: newMail.setMessage(new MimeMessage(originalMail.getMessage()) { protected void updateHeaders() throws MessagingException { if (getMessageID() == null) super.updateHeaders(); else { modified = false; } } }); IMHO adding a copyonwrite proxy over the MimeMessageWrapper we can remove also the deepcopy in the abstractRedirect and achieve better performance than now, btw I don't care how we decide to fix it. I just want it to be fixed. Stefano - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [jira] Commented: (JAMES-421) MailImpls sharing MimeMessage's!
> IMHO the DEFAULT behaviour should be the deep copy. I disagree, what we're trying to do is to manage routing the message. Message can be routed down two different routes to its destination, but is still the same message. The default should be the least resource intensive. In fact I would tend to say that the issue is not technical, but one of documentation. The docs should clearly explain that and any changes should be made only to ensure that there is a simple and consistent method which can be used to branch the message itself. d. *** The information in this e-mail is confidential and for use by the addressee(s) only. If you are not the intended recipient (or responsible for delivery of the message to the intended recipient) please notify us immediately on 0141 306 2050 and delete the message from your computer. You may not copy or forward it or use or disclose its contents to any other person. As Internet communications are capable of data corruption Student Loans Company Limited does not accept any responsibility for changes made to this message after it was sent. For this reason it may be inappropriate to rely on advice or opinions contained in an e-mail without obtaining written confirmation of it. Neither Student Loans Company Limited or the sender accepts any liability or responsibility for viruses as it is your responsibility to scan attachments (if any). Opinions and views expressed in this e-mail are those of the sender and may not reflect the opinions and views of The Student Loans Company Limit ed. This footnote also confirms that this email message has been swept for the presence of computer viruses. ** - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [jira] Commented: (JAMES-421) MailImpls sharing MimeMessage's!
> This is the usual deep versus shallow copy discussion, and > actually both methods make sense for different purposes. > I think we should provide a deep-duplicate or such method, as > there might be someone out there, who has coded against the > shallow copy behaviour. IMHO the DEFAULT behaviour should be the deep copy. We can provide a method that share the MimeMessage but the user should understand what he's doing. Currently also james core developers used it the wrong way because it's too easy to do the wrong thing with the current behaviour. Mailet writers can do the wrong thing too easily. Stefano - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [jira] Commented: (JAMES-421) MailImpls sharing MimeMessage's!
This is the usual deep versus shallow copy discussion, and actually both methods make sense for different purposes. I think we should provide a deep-duplicate or such method, as there might be someone out there, who has coded against the shallow copy behaviour. --Søren On Friday 09 September 2005 09:45, Stefano Bagnara (JIRA) wrote: > [ > http://issues.apache.org/jira/browse/JAMES-421?page=comments#action_1232300 >6 ] > > Stefano Bagnara commented on JAMES-421: > --- > > Here is what it happens in my test: > I send a mail "mail1" to [EMAIL PROTECTED] and [EMAIL PROTECTED] and "original > body" body. The first matcher split the mail1 by duplicating it to > "mail1-!27120": it then remove a recipient from mail1 and the other from > mail1-!27120. mail1 run to the fist MyMailet that change its body to new > text 0. Both Mails run then through the second MyMailet that should change > the body of one mail to "new text 1" and the body of the other mail to "new > text 2". I then print the 2 bodies: I have 2 messages with "new text 2" > body and NO ONE with "new text 1". > > - AbstractRedirect does correctly clone the MimeMessage after the call to > MailImpl.duplicate - Many mailets just use sendMail with the shared > MimeMessage: we should add a comment to mailetContext sendMails to let the > use know that we lock the MimeMessage until we processed it and we never > change it. - LinearProcessor (after partial matching) does duplicate and > this way sends multiple Mails with sharing MimeMessage to the following > mailets. > > We could solve the issue by cloning the MimeMessage in LinearProcessor but > this is too easy to exploit: IMHO we should change the duplicate so that we > wrap the object with a "CopyOnWrite" shield: so we can safely share the > MimeMessage also when storing it and after multiple operations. > > > MailImpls sharing MimeMessage's! > > > > > > Key: JAMES-421 > > URL: http://issues.apache.org/jira/browse/JAMES-421 > > Project: James > > Type: Bug > > Components: James Core > > Versions: 2.3.0, 2.2.0 > > Reporter: Stefano Bagnara > > Assignee: Stefano Bagnara > > Priority: Critical > > Fix For: 2.3.0 > > Attachments: LinearProcessorTest.java > > > > LinearProcessor match a single recipient for a 2 recipient mail. > > it run "MailImpl.duplicate". duplicate DOES NOT clone the "MimeMessage". > > The following mailet will handle 2 different MailImpl sharing the same > > MimeMessage. Attached is the proving test. -- Søren Hilmer, M.Sc. R&D manager Phone: +45 72 30 64 00 TietoEnator IT+ A/S Fax:+45 72 30 64 40 Ved Lunden 12 Direct: +45 72 30 64 57 DK-8230 Åbyhøj Email: soren.hilmer tietoenator.com - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
[jira] Commented: (JAMES-421) MailImpls sharing MimeMessage's!
[ http://issues.apache.org/jira/browse/JAMES-421?page=comments#action_12323006 ] Stefano Bagnara commented on JAMES-421: --- Here is what it happens in my test: I send a mail "mail1" to [EMAIL PROTECTED] and [EMAIL PROTECTED] and "original body" body. The first matcher split the mail1 by duplicating it to "mail1-!27120": it then remove a recipient from mail1 and the other from mail1-!27120. mail1 run to the fist MyMailet that change its body to new text 0. Both Mails run then through the second MyMailet that should change the body of one mail to "new text 1" and the body of the other mail to "new text 2". I then print the 2 bodies: I have 2 messages with "new text 2" body and NO ONE with "new text 1". - AbstractRedirect does correctly clone the MimeMessage after the call to MailImpl.duplicate - Many mailets just use sendMail with the shared MimeMessage: we should add a comment to mailetContext sendMails to let the use know that we lock the MimeMessage until we processed it and we never change it. - LinearProcessor (after partial matching) does duplicate and this way sends multiple Mails with sharing MimeMessage to the following mailets. We could solve the issue by cloning the MimeMessage in LinearProcessor but this is too easy to exploit: IMHO we should change the duplicate so that we wrap the object with a "CopyOnWrite" shield: so we can safely share the MimeMessage also when storing it and after multiple operations. > MailImpls sharing MimeMessage's! > > > Key: JAMES-421 > URL: http://issues.apache.org/jira/browse/JAMES-421 > Project: James > Type: Bug > Components: James Core > Versions: 2.3.0, 2.2.0 > Reporter: Stefano Bagnara > Assignee: Stefano Bagnara > Priority: Critical > Fix For: 2.3.0 > Attachments: LinearProcessorTest.java > > LinearProcessor match a single recipient for a 2 recipient mail. > it run "MailImpl.duplicate". duplicate DOES NOT clone the "MimeMessage". > The following mailet will handle 2 different MailImpl sharing the same > MimeMessage. > Attached is the proving test. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - 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]
[jira] Commented: (JAMES-421) MailImpls sharing MimeMessage's!
[ http://issues.apache.org/jira/browse/JAMES-421?page=comments#action_12323003 ] Stefano Bagnara commented on JAMES-421: --- It doesn't matter what Mail is supposed to do: if this is not a bug in MailImpl it is a bug in LinearProcessor! And a critical one: James send a different mail from what it's supposed to send. > MailImpls sharing MimeMessage's! > > > Key: JAMES-421 > URL: http://issues.apache.org/jira/browse/JAMES-421 > Project: James > Type: Bug > Components: James Core > Versions: 2.3.0, 2.2.0 > Reporter: Stefano Bagnara > Assignee: Stefano Bagnara > Priority: Critical > Fix For: 2.3.0 > Attachments: LinearProcessorTest.java > > LinearProcessor match a single recipient for a 2 recipient mail. > it run "MailImpl.duplicate". duplicate DOES NOT clone the "MimeMessage". > The following mailet will handle 2 different MailImpl sharing the same > MimeMessage. > Attached is the proving test. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - 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]
[jira] Commented: (JAMES-421) MailImpls sharing MimeMessage's!
[ http://issues.apache.org/jira/browse/JAMES-421?page=comments#action_12322983 ] Noel J. Bergman commented on JAMES-421: --- Mail objects are just addressing carriers for the message. They are not supposed to clone the message, since we may simply be effecting routing within the pipeline. The overhead would be huge if every split in routing caused a clone of the underlying message. > MailImpls sharing MimeMessage's! > > > Key: JAMES-421 > URL: http://issues.apache.org/jira/browse/JAMES-421 > Project: James > Type: Bug > Components: James Core > Versions: 2.3.0, 2.2.0 > Reporter: Stefano Bagnara > Assignee: Stefano Bagnara > Priority: Critical > Fix For: 2.3.0 > Attachments: LinearProcessorTest.java > > LinearProcessor match a single recipient for a 2 recipient mail. > it run "MailImpl.duplicate". duplicate DOES NOT clone the "MimeMessage". > The following mailet will handle 2 different MailImpl sharing the same > MimeMessage. > Attached is the proving test. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - 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]