You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by "Markus Wiederkehr (JIRA)" <se...@james.apache.org> on 2008/09/09 17:03:45 UTC

[jira] Created: (MIME4J-72) Provide a means to dispose a Message

Provide a means to dispose a Message
------------------------------------

                 Key: MIME4J-72
                 URL: https://issues.apache.org/jira/browse/MIME4J-72
             Project: Mime4j
          Issue Type: Improvement
    Affects Versions: 0.5
            Reporter: Markus Wiederkehr


Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.

If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.

For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.

Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Bernd Fondermann (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12637792#action_12637792 ] 

Bernd Fondermann commented on MIME4J-72:
----------------------------------------

I reviewed both patches and I think this kind of resource managment is not a good thing.

Disadvantages:
+ It pollutes the whole framework, by introducing dispose() and if(disposed) checks everywhere and although only TempFile*Body is concerned. 
+ dispose state removes indempotency. two method calls yield different results, if dispose() was called in between. but there is no way to check if a part has been disposed
+ delete candidate files are held in a static final container. if one file is to be deleted, all files failing deletion are visited (again (and again)) in a synchonized block. this is bad. it introduces a severe bottleneck for the whole framework.

Prososal:
+ Introduce 
public interface ManagedResource {
    ResourceManager getResourceManager();
}
only on TempFile*Body.
The ResourceManager should then be responsible for managing the tempory files. tracking, copying, deleting, whatever.

Advantages:
+ ResourceManager can do all the complicated things we now try to objects which should not be concerned with resource management and without polluting our existing framework interfaces.
+ We make this concern separately testable, and exchangable.
+ ResourceManager can grow without affecting other parts of the framework and changing interfaces
+ ResourceManager must not rely on calls to dispose() to efffectively collect disposed resources (removing temp files), this can be done by a scheduler or a method call etc.
+ no keeping track of the disposed flag in every method call





> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch, mime4j-dispose-finalize.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Markus Wiederkehr (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638433#action_12638433 ] 

Markus Wiederkehr commented on MIME4J-72:
-----------------------------------------

Sorry here is that link again without the trailing colon: http://docs.google.com/View?docid=dffxznxr_1nmsqkz

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: entityvisitor.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Markus Wiederkehr (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12637850#action_12637850 ] 

Markus Wiederkehr commented on MIME4J-72:
-----------------------------------------

@Bernd:

dispose() should free any resources that cannot be reclaimed by the garbage collector. This implies that dispose() essentially destroys the object and it can no longer be used afterwards. So idempotency cannot be an issue here. It is perfectly okay for an object to behave unpredictably once disposed. You would not expect an InputStream to work as it did before once you have closed it, would you?

The intention is to dispose an object and leave it to the garbage collector. If you continue to use it your are doing something wrong.

As for the pollution of the framework, this is a valid point. But the disposed flag is entirely optional and can be removed. I will provide a patch for that shortly.

Your third concern was "all files failing deletion are visited (again (and again))". I don't see where this happens. There is no dispose call inside a finally block (the super.dispose() in TempFileBinaryBody only sets the parent to null, there is no recursion, loop or anything).

Considering your proposal of a ResourceManager on TempFile*Body.. TempFileTextBody and TempFileBinaryBody aren't even public classes.. And the use case is to simply dispose a message and free all temp files held by it or its parts. With my approach you simply call message.dispose() and be done with it. With your approach you would first have to traverse the message hierarchy down to the TempFile*Bodys, obtain the ResouceManager and then free the resources - seems like a lot of work to me.

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Resolved: (MIME4J-72) Provide a means to dispose a Message

Posted by "Oleg Kalnichevski (JIRA)" <se...@james.apache.org>.
     [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski resolved MIME4J-72.
-------------------------------------

       Resolution: Fixed
    Fix Version/s: 0.5

The latest patch proposed by Markus tightly couples generic Part interface with two of its concrete implementations (Message, Multipart) through the Visitor interface, which is conceptually ... well... very wrong. I cannot commit such a patch. Better no visitor at all. 

I committed mime4j-dispose-no-clutter.patch with some minor tweaks. Anyone willing to continue working on the Visitor support for Message classes is welcome to take over from here.

Oleg

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>             Fix For: 0.5
>
>         Attachments: mime4j-bodywalker-disposable.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Reopened: (MIME4J-72) Provide a means to dispose a Message

Posted by "Oleg Kalnichevski (JIRA)" <se...@james.apache.org>.
     [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski reopened MIME4J-72:
-------------------------------------


I second that. The use of disposed flag in Multipart, Message and Entity in its current form is a problem. It is very error prone and very unpleasantly intrusive.

@Bernd
Could you please outline how ResourceManager interface may look like?

Alternatively, the problem may also be solved by traversing the hierarchy of message parts and calling #dispose only on those instances that implement optional Disposable interface. Moreover, a generic mechanism to traverse hierarchy of message parts, probably in a form of support the Visitor pattern, may be a worthwhile addition.

Oleg

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch, mime4j-dispose-finalize.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Stefano Bagnara (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12636007#action_12636007 ] 

Stefano Bagnara commented on MIME4J-72:
---------------------------------------

The patch looks fine. I have issues with the "finalize" methods. In JAMES we removed the finalizes because the JVM does not guarantee that finalize will be called, and having a finalize that "sometimes" works can lead to people thinking it works always and that manual disposition is not required. Instead I would prefer to document that to release resources you have to call dispose and if you don't do that you get you can have leaks (file/memory).
Another option would be to have a check in finalize to log an error when finalize is called on a non disposed object, so to alert users that do not read javadocs.

Opinions?

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch, mime4j-dispose-finalize.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Assigned: (MIME4J-72) Provide a means to dispose a Message

Posted by "Robert Burrell Donkin (JIRA)" <se...@james.apache.org>.
     [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Robert Burrell Donkin reassigned MIME4J-72:
-------------------------------------------

    Assignee: Robert Burrell Donkin

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Oleg Kalnichevski (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638548#action_12638548 ] 

Oleg Kalnichevski commented on MIME4J-72:
-----------------------------------------

@Markus

> I like separation of concerns but why should something as elementary as object destruction be a separate concern? 

Why _all_ MIME classes (Message, Entity, Part) must implement an extra method that applicable to a few Part implementations?

> You dispose of a message for the same reasons you close an I/O stream. 

Not all Message instances are generated by reading data from an I/O stream. Why _all_ of them should be disposable?

> It has to be an inherent part of the API. 

Why?

> Therefore it is impossible to determine if binary part #3 belongs to multipart #1 or multipart #2

They would have different parents, wouldn't they?

> If I wanted to do instance-of I would not need a visitor in the first place

What is the point of trying to make a Swiss Army knife out of everything and failing in the process? Your implementation currently does seem to correctly handle embedded messages because it does not check the type of individual Parts in a Multipart entities.

Clearly, one Visitor interface just can't satisfy everyone. I suggest that we decouple the traversal logic from Message / Entity / Part API, so one could have different iterator / walker implementations invoking different Visitor interfaces on items they traverse.

Oleg

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: entityvisitor.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Markus Wiederkehr (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638462#action_12638462 ] 

Markus Wiederkehr commented on MIME4J-72:
-----------------------------------------

Oleg, here are a few more thoughts, please bear with me..

dispose method: unless you decide to keep the entire message in memory you will _always_ have the need to free the backing resources when the message is no longer needed. Regardless of what backing mechanism you want to use. Therefore the typical use case looks like this:

Message message = acquireMessage();
try {
    doSomethingFancy(message);
}
finally {
    disposeMessage(message);
}

Clearly message.dispose() is more convenient than message.accept(new DisposingEntityVisitor()).

So this is not something exotic. You dispose of a message for the same reasons you close an I/O stream. It has to be an inherent part of the API.

Regarding the visitor, Oleg's vs Markus'.. Please consider a message like this one:

message
      multipart #1
            text part #1
            multipart #2
                  binary part #1
                  binary part #2
            binary part #3

I believe that your visitor cannot be used to copy the structure of this message. It gets notified of the entities and bodies in the order they appear. Therefore it is impossible to determine if binary part #3 belongs to multipart #1 or multipart #2.

Besides, what good is a visitor if I have to do all the instance-of checks by myself. If I wanted to do instance-of I would not need a visitor in the first place.

After all a visitor is only a crutch for languages that don't have a multiple dispatch mechanism. So the visitor should inform you on the concrete leaf classes as good as possible.

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: entityvisitor.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Oleg Kalnichevski (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638653#action_12638653 ] 

Oleg Kalnichevski commented on MIME4J-72:
-----------------------------------------

@Markus

> It's your project after all 

It is not.

> Even the Java example in the Wikipedia article on the visitor pattern works exactly as my version does.

Congratulations. 

I apologize for being such a simpleton. I'll commit some variation of your mime4j-dispose-no-clutter.patch patch, and proceed with the 0.5, after which you are very welcome to take over.

Oleg

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-bodywalker-disposable.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Markus Wiederkehr (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638270#action_12638270 ] 

Markus Wiederkehr commented on MIME4J-72:
-----------------------------------------

Please reconsider what the original issue was. A message allocates temporary files that are not deleted until the jvm terminates. That was the situation with 0.4 and the reason I filed this trouble ticket.

So clearly it should be possible to free these resources once a Message object is no longer needed. This should be a fundamental part of the API and in my opinion it should be _simple to use_.

message.accept(new DisposingEntityVisitor()) is not very straight forward but message.dispose() is.

Of course you could add a convenience method in Message and do the visitor thing there. That would essentially make message disposable again.

But then I would like to have the same feature in BodyPart because if I remove a BodyPart from it's parent Entity ultimately I want to get rid of it. So BodyPart should be disposable.

Please don't get me wrong. I like the visitor idea. Many cool things could be done with a visitor such as copying and manipulating a message. But disposing a message and all its resources is such a fundamental thing that I don't see that you really need a visitor for it.

But speaking of manipulating a message. Would not a visitor that recognized Message and Multipart be a bit more powerful?

Something like this:

public interface BodyVisitor {
    void visitMessage(Message message);
    void visitMultipart(Multipart multipart);
    void visitBody(Body body);
}

public abstract class AbstractBodyVisitor implements BodyVisitor {
    public void visitMessage(Message message) {
        beginMessage(message);
        Body body = message.getBody();
        if (body != null) {
            body.accept(this);
        }
        endMessage(message);
    }

    public void visitMultipart(Multipart multipart) {
        beginMultipart(multipart);
        List bodyParts = multipart.getBodyParts();
        for (Iterator it = bodyParts.iterator(); it.hasNext();) {
            Body body = ((BodyPart) it.next()).getBody();
            if (body != null) {
                body.accept(this);
            }
        }
        endMultipart(multipart);
    }

    protected void beginMessage(Message message) {
    }
    
    protected void endMessage(Message message) {
    }

    protected void beginMultipart(Multipart multipart) {
    }
    
    protected void endMultipart(Multipart multipart) {
    }
}

Note that the traversal happens in the abstract implementation instead of in Message and Multipart.

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: entityvisitor.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Markus Wiederkehr (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638750#action_12638750 ] 

Markus Wiederkehr commented on MIME4J-72:
-----------------------------------------

Oleg, please add a check if body is null to Entity.dispose().

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>             Fix For: 0.5
>
>         Attachments: mime4j-bodywalker-disposable.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Closed: (MIME4J-72) Provide a means to dispose a Message

Posted by "Robert Burrell Donkin (JIRA)" <se...@james.apache.org>.
     [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Robert Burrell Donkin closed MIME4J-72.
---------------------------------------

    Resolution: Fixed

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Updated: (MIME4J-72) Provide a means to dispose a Message

Posted by "Oleg Kalnichevski (JIRA)" <se...@james.apache.org>.
     [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski updated MIME4J-72:
------------------------------------

    Attachment: entityvisitor.patch

Here is an improved version of my previous patch
* EntityVisitor interface replaced BodyVisitor; EntityVisitorcan be used to interact with both Body and Entity objects
* Made Disposable interface optional. Only TempFile*Body classes implement Disposable
* Added DisposingEntityVisisor implementation to release resources held by disposable Body objects

I hope this should satisfy all parties involved. We can think of something more sophisticated in the course of 0.6 development

Folks,

Please do find a minute to review the patch as it currently blocks the 0.5 release.

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: entityvisitor.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Updated: (MIME4J-72) Provide a means to dispose a Message

Posted by "Markus Wiederkehr (JIRA)" <se...@james.apache.org>.
     [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Markus Wiederkehr updated MIME4J-72:
------------------------------------

    Attachment: mime4j-disposable.patch

Please consider the attached patch for inclusion in the next release.

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>         Attachments: mime4j-disposable.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Markus Wiederkehr (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12631719#action_12631719 ] 

Markus Wiederkehr commented on MIME4J-72:
-----------------------------------------

Thanks for including this. It looks like you forgot to commit the Disposable interface though.

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Markus Wiederkehr (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12636015#action_12636015 ] 

Markus Wiederkehr commented on MIME4J-72:
-----------------------------------------

I understand your concern with finalizers being the flawed thing they are. I wanted to provide the same kind of safety net you can find in various java.io classes. But of course this code also is from Java 1.0 and it might be better not to imitate it.

So if you strip out the finalize parts of my patch and apply the rest it's fine by me.

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch, mime4j-dispose-finalize.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Updated: (MIME4J-72) Provide a means to dispose a Message

Posted by "Oleg Kalnichevski (JIRA)" <se...@james.apache.org>.
     [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski updated MIME4J-72:
------------------------------------

    Attachment: mime4j-bodywalker-disposable.patch

* Body interface no longer imposes a particular Visitor contract
* Added BodyWalker classes intended to traverse the hierarchy of Body objects contained in a message and to invoke a BodyVisitor interface on visited Body instances

@Markus,

Feel free to provide a full blown MessageWalker impl with all possible bells and whistles

Oleg



> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-bodywalker-disposable.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Updated: (MIME4J-72) Provide a means to dispose a Message

Posted by "Oleg Kalnichevski (JIRA)" <se...@james.apache.org>.
     [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski updated MIME4J-72:
------------------------------------

    Attachment:     (was: entityvisitor.patch)

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Stefano Bagnara (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12636832#action_12636832 ] 

Stefano Bagnara commented on MIME4J-72:
---------------------------------------

I applied the patch without finalizers (commented out).
I leave the issue open so hopefully someone else will review before releasing 0.5 and tell his opinion about the use of finalizers.

Thank you Markus!

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch, mime4j-dispose-finalize.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Stefano Bagnara (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638765#action_12638765 ] 

Stefano Bagnara commented on MIME4J-72:
---------------------------------------

+1: I think we should release 0.5 with the dispose method. It is not so different from the "close" in the InputStream hierarchy, so I feel it easy to understand and simple. If we find a better solution (e.g: a visitor based solution that we all like) we can put it in 0.6. Mime4j is young, and we have to experiment and to release.

@Markus: your contributions are really very welcome, don't be discouraged if 0.5 is shipped with the first solution! The good news is that 0.5 will have disposable message and we have to thank you for this!

@Oleg: You are always very effective, thank you.

I propose this issue to be closed for 0.5 as is, and that if we want to try some visitor solution in 0.6 we should add a new issue.

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>             Fix For: 0.5
>
>         Attachments: mime4j-bodywalker-disposable.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Updated: (MIME4J-72) Provide a means to dispose a Message

Posted by "Markus Wiederkehr (JIRA)" <se...@james.apache.org>.
     [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Markus Wiederkehr updated MIME4J-72:
------------------------------------

    Attachment: mime4j-dispose-finalize.patch

mime4j-dispose-finalize.patch is another patch for this issue I would like to be considered. I hope it can be included in 0.5.

The main changes are:
*) AbstractBody, Entity and Multipart have a boolean flag named 'disposed' to keep track if they have been disposed. Invoking dispose() a second time has no effect.
*) Once disposed all other methods throw a runtime exception (explicit check in setters, implicit NPEs in getters).
*) Most importantly AbstractBody, Entity and Multipart also have a finalizer that invokes dispose(). This is the same kind of safety net that is used by FileInputStream to invoke close(), for example.
*) SimpleTempStorage now actually tries to delete the backing file if delete() is invoked. If the deletion fails (e.g. because a stream is still open) the file is remembered for later deletion.
*) Additional unit tests

A side effect of this patch is that getParent() and setParent() are removed from TempFileBinaryBody because this is already handled by AbstractBody.

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch, mime4j-dispose-finalize.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Oleg Kalnichevski (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638407#action_12638407 ] 

Oleg Kalnichevski commented on MIME4J-72:
-----------------------------------------

@Markus

There is a lot of points to discuss and to disagree about. Let me mention the most important ones for now. My main reservation about the API changes you are proposing is that they go counter to the principle of separation of concerns. (1) Intermediate storage, disposal of temporary resources, sophisticated object coping are _implementation_ aspects that ideally should not pollute Message / Entity / Body API, which is meant to be _generic_. Ideally they should be handled separately without over-complicating the generic API. (2) I also cannot fully agree that it is the visitor concern to decide what objects to visit. It breaks a common pattern and just does not seem right.

Please do not feel discouraged. Disagreements are very common on this list. I and Stefan sometimes spend _weeks_ disagreeing :-/

@All

Here's my suggestion. I would like to commit my version of the patch _for now_. It addresses the concerns raised by Bernd (which I fully share) while still allowing to take care of resource deallocation and message cloning Markus needs, albeit with a bit of extra coding.

Let's get 0.5 released rather sooner than later and then carefully consider further improvements without the pressure of holding back a much needed release. 

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: entityvisitor.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Updated: (MIME4J-72) Provide a means to dispose a Message

Posted by "Markus Wiederkehr (JIRA)" <se...@james.apache.org>.
     [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Markus Wiederkehr updated MIME4J-72:
------------------------------------

    Attachment: mime4j-dispose-visitor.patch

This patch is based on Oleg's latest one with a few changes:

* Message, Multipart and BodyPart are disposable, _but not all Body implementations have to be_.
* The visitor can now handle Message, Multipart, BodyPart and Body
* Added an abstract visitor implementation that traverses down the object hierarchy
* DisposingEntityVisitor is now a singleton
* some cleanup in TempFileTextBody and TempFileBinaryBody

Using this visitor it is now also easy to copy a message.

I hope this an acceptable solution.

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: entityvisitor.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Robert Burrell Donkin (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12631563#action_12631563 ] 

Robert Burrell Donkin commented on MIME4J-72:
---------------------------------------------

Committed. Many thanks.

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Stefano Bagnara (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638370#action_12638370 ] 

Stefano Bagnara commented on MIME4J-72:
---------------------------------------

@Markus: About your 2nd point (The visitor can now handle Message, Multipart, BodyPart and Body )

This sounds weaker than Oleg's one.

E.g: Multipart is not defined by the basic MIME specification. In rfc1521 we have "message", "body part", "entity" and "body" defined. It also define that "message" and "body part" are "entities", so once we defined "entity" and "body" we can visit anything. Everything else is just a specialization of entity or body (or both). 

Adding anything beyond what is defined in rfc1521 to a generic visitor make it no more extensible or tied to specific extensions.

AFAIUI "Oleg's Visitor" already allow you to traverse the whole structure (multipart included), what feature are you pursuing with your changes?

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: entityvisitor.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Updated: (MIME4J-72) Provide a means to dispose a Message

Posted by "Markus Wiederkehr (JIRA)" <se...@james.apache.org>.
     [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Markus Wiederkehr updated MIME4J-72:
------------------------------------

    Attachment: mime4j-dispose-no-clutter.patch

Please consider this patch that removes all the code cluttering but leaves the general mechanism as it is.

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Markus Wiederkehr (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638623#action_12638623 ] 

Markus Wiederkehr commented on MIME4J-72:
-----------------------------------------

@Oleg

By now it seems to be clear that we won't be able to reach a consensus. So for now this will be my last comment on the issue. It's your project after all and you can do with it what you want. I also don't want to stand in the way of your upcoming 0.5 release.

But let me comment on a few of your statements:

> Not all Message instances are generated by reading data from an I/O stream. Why _all_ of them should be disposable?

Because it is convenient. The dispose method in Message I proposed is nothing more that a convenience method that uses your disposing visitor. You don't have to call dispose if you can be sure that your message resides entirely in memory.

If such a small convenience method is such a bit thorn in your eye, have you considered replacing Message.writeTo() with a visitor implementation? That would only be consequent since you don't always need writeTo() either.

> Your implementation currently does seem to correctly handle embedded messages because it does not check the type of individual Parts in a Multipart entities.

My implementation correctly handles embedded messages because of a sophisticated feature called polymorphism, you might want to check on it.

> Added BodyWalker classes intended to traverse the hierarchy of Body objects contained in a message and to invoke a BodyVisitor interface on visited Body instances

So now I have to write "new BodyWalker(new DisposingBodyVisitor()).walk(message)" to get rid of my message? This is getting better and better..

> (Markus) If I wanted to do instance-of I would not need a visitor in the first place

Your BodyWalker is exactly what I meant by that. It uses instance-of checks instead of simply utilizing polymorphism. So it is not even a true visitor in the meaning of the classical design pattern.

Even the Java example in the Wikipedia article on the visitor pattern (http://en.wikipedia.org/wiki/Visitor_pattern) works exactly as my version does. Object traversal is done in the visitor implementation (PrintVisitor.visitCar()) by invoking _accept_ (not visit) on the child element to visit next.

Markus

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-bodywalker-disposable.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Stefano Bagnara (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12631735#action_12631735 ] 

Stefano Bagnara commented on MIME4J-72:
---------------------------------------

@Markus: I just added the Disposable interface! Thank you again!

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Updated: (MIME4J-72) Provide a means to dispose a Message

Posted by "Oleg Kalnichevski (JIRA)" <se...@james.apache.org>.
     [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski updated MIME4J-72:
------------------------------------

    Attachment:     (was: bodyvisitor.patch)

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Stefano Bagnara (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638259#action_12638259 ] 

Stefano Bagnara commented on MIME4J-72:
---------------------------------------

I don't have a strong preference between the dispose method in the whole object tree (current trunk) and the entityVisitor+Disposable solution. The EntityVisitor solution is much more elegant and probably flexible but may be a little harder to understand at first.

I reviewed the patch from Oleg (entityvisitor.patch) and it looks good to me.

This is to say that I'm fine with current trunk (by Markus) and with entityvisitor (by Oleg) while I don't like (and I don't fully understand ATM) the ResourceManager+ManagedResource proposal (by Bernd).

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: entityvisitor.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Updated: (MIME4J-72) Provide a means to dispose a Message

Posted by "Oleg Kalnichevski (JIRA)" <se...@james.apache.org>.
     [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski updated MIME4J-72:
------------------------------------

    Attachment: bodyvisitor.patch

This patch implements the visitor pattern for the Body interface. This should pretty much eliminate the need for each and every component of a message to implement Disposable. One can simply visit all Body instances encapsulated in a Message and call #dispose on those that implement Disposable.

Oleg

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: bodyvisitor.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Markus Wiederkehr (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638431#action_12638431 ] 

Markus Wiederkehr commented on MIME4J-72:
-----------------------------------------

@Oleg

Indeed there seems to be a lot to disagree about. If disagreement is so common on this list then please let me continue ;-)

Regarding the dispose method.. With finalizers being the flawed thing they are explicit close/dispose/destroy methods are the closest thing in Java we have to a destructor. Please take a look at http://docs.google.com/View?docid=dffxznxr_1nmsqkz, a proposal for Java 7 from Joshua Bloch. That interface Foo he talks about corresponds to the Disposable interface I have introduced in Mime4j.

So this is not something I have invented or has never been heard of before. I like separation of concerns but why should something as elementary as object destruction be a separate concern?

Regarding the visitor. Please take a look at the GoF Design Patterns book page 339, the section "Who is reponsible for traversing the object structure?" You can put the traversal in, I quote, "the object structure, in the visitor or in a separate iterator object". And later "The main reason to put the traversal in the visitor is to implement a particular complex traversal...". That complex traversal is what we need and there is no common pattern.

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: entityvisitor.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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


[jira] Commented: (MIME4J-72) Provide a means to dispose a Message

Posted by "Markus Wiederkehr (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/MIME4J-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638384#action_12638384 ] 

Markus Wiederkehr commented on MIME4J-72:
-----------------------------------------

There are two issues that are somewhat related.

1) With Oleg's version the visitor cannot decide how deep it wants to go into the message hierarchy. To give you an example let's assume you are interested in processing multipart/mixed messages but you wish to ignore multipart/alternative or multipart/related. Or assume you want to process PDF attachments but not in inner (forwarded) messages.

With my implementation the visitor decides whether to visit inner Bodies / Entities or not. Oleg's version gives the visitor no choice because the traversal is hard-coded into Message / Multipart.

2) Regardless of RFC 1521 Message/Multipart/BodyPart/Body is the structure of a Mime4j Message object. Let's assume you wanted to copy a message. It could be done like this:

class CopyVisitor extends AbstractEntityVisitor {
    private Stack stack = new Stack();

    public void beginMessage(Message message) {
        Message copy = new Message();
        copyHeader(message, copy);

        if (!stack.isEmpty())
            ((Entity) stack.peek()).setBody(copy);
        stack.push(copy);
    }

    public void endMessage(Message message) {
        stack.pop();
    }

    // analogous for Multipart and BodyPart.

    public void visitBody(Body body) {
        // called for all body implementations except Message and Multipart
        Body copy = copyBody(body);
        ((Entity) stack.peek()).setBody(copy);
    }

    private Header copyHeader(Entity source, Entity target) { ... } 
    private Body copyBody(Body body) { ... } 
}

You could also write visitors that copy certain bodies of a message, remove other bodies or replace certain bodies with post-processed versions.

Of course this could also be done with a visitor that distinguishes between Entity and Body alone. But it would require a lot more is-instanceof checks..

> Provide a means to dispose a Message
> ------------------------------------
>
>                 Key: MIME4J-72
>                 URL: https://issues.apache.org/jira/browse/MIME4J-72
>             Project: JAMES Mime4j
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Markus Wiederkehr
>            Assignee: Robert Burrell Donkin
>         Attachments: entityvisitor.patch, mime4j-disposable.patch, mime4j-dispose-finalize.patch, mime4j-dispose-no-clutter.patch, mime4j-dispose-visitor.patch
>
>
> Currently an org.apache.james.mime4j.message.Message uses temporary files to store text and binary attachments of the message. Unfortunately a Message cannot be disposed of explicitly. Even when it eventually gets garbage collected the temp files continue to exist until the VM exits.
> If the VM runs for a long time and a lot of e-mails get processed this can become a major problem.
> For this reason I think that class Entity and interface Body should both have a method to dispose of the object. Multipart should dispatch a dispose-call to its list of body parts. A BodyPart should dispose of its body and concrete Body implementation such as TempFileTextBody should ultimately invoke delete() on the backing TempFile.
> Last but not least SimpleTempStorage$SimpleTempFile should not silently ignore delete-calls.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


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