Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: 2.3.0
-
Fix Version/s: 2.3.0
-
Component/s: James Core
-
Labels:None
Description
The change introduced in rc2 to fix the file leaks has produced an NPE in a custom mailet.
I suggest to revert the setMessage changes removing the dispose() call for the old message and to put the dispose code in the bundled calling mailets.
This is not a good fix in the long term, but for 2.3.0 I would prefer to keep compatibility with 2.2.0 and to have a leak instead of NPE (difficult to understand by most users).
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
This is a possible fix to the problem.
Few changes are on the tests to solve some temporary file not release (24 before the fix, 20 after the fix).
Now that I wrote it I'm not sure about this in 2.3: they are a lot of changes... maybe it is better to add a check in setMessage to see if the new message is the same of the old one...
I have to think more about this...
I agree that it would be better to check in setMessage if this.message == message.
None of these is a perfect solution, since the explicit dispose call reintroduces the problems that exist in other languages with explicit deallocation: how to know when it is safe to deallocate (and clean-up). What if someone were to call getMessage, put the old message object into a new Mail, and put a new message in the old Mail?
But the least objectionable change for now is probably the check in setMessage. The logic for that one is clear: if we are setting the message to the one we already have, do nothing.
We agreed to fix this with the small "identity check" fix.
Applied to both 2.3 branch and trunk.
Closing issue fixed in released version.
I believe the NPE could be thrown when saving the message after having called SMIMECheckSignature when "stripSignature && strippedMessage != null" because it calls setMessage with the same message.