On 11/30/08, Markus Wiederkehr <[EMAIL PROTECTED]> wrote: > On Sun, Nov 30, 2008 at 11:59 PM, Robert Burrell Donkin > <[EMAIL PROTECTED]> wrote: >> http://svn.apache.org/repos/asf/james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/storage/MultiReferenceStorage.java >> uses method based synchronization to protect the reference counting >> variable. i wonder whether this is the best approach. (i tend to >> prefer explicit monitors that cannot leak for defensive reasons.) >> >> in particular >> >> public synchronized void delete() { >> if (referenceCounter == 0) >> return; >> >> referenceCounter--; >> >> if (referenceCounter == 0) { >> storage.delete(); >> } >> } >> >> 1. storage is an interface and so storage.delete() represents a leak >> of the thread of execution. i wonder whether it is possible for >> competing storage deletes to produce a deadlock. >> 2. storage.delete() may be a slow operation. the locking appears to be >> protecting only the reference counting. if so then the delete may not >> need to be synchronized, >> >> it might be possible to avoid explicit synchronization by using >> AtomicInteger instead >> >> opinions? > > Robert, thank you very much for committing my patches so fast. > > Regarding the problem with the interface and the leak you describe, I > think I have to read up on that one..
Basically, the class has no control over the implementation and cannot stop deadlock conditions etc > But for delete() possibly being a slow operation, how about using > internaynchronization instead? I mean determine if the storage has > to be deleted in a "synchronized this" block and do the actual > deletion outside of the block.. +1 It's also worthwhile thinking about whether it would be better to use a different monitor (rather than this). Not sure whether the additional complex would be a good trade in this case. - Robert > > Markus > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
