Bernd Fondermann ha scritto:
> What types of exceptions do you get? NPE?
>
> if it wasn't for a ThreadLocal, I'd say that this method's init is not
> atomic, if fieldInstance is NULL:
>
> static protected void setInstance(CommandStateManager conditionManager)
> {
> if (null == fieldInstance)
> fieldInstance = new ThreadLocal();
> fieldInstance.set(conditionManager);
> }
>
> It leaves ThreadLocal uninitialized but accessible for a little
> moment. if the method got interrupted in between and getInstance()
> gets executed, this might result in a NPE.
Good catch!!
> But maybe this case will never occur because of the special nature of
> ThreadLocals?
I don't think so, but your pointer brought me to look for callers of
that code, and here it is:
/**
* Method execute executes a Block within the context of a new
ConditionManager.
* @param mail
* @param block
* @return Object
* @throws SieveException
*/
protected Object execute(MailAdapter mail, Block block) throws
SieveException {
// Switch to a new ConditionManager
ConditionManager oldManager = ConditionManager.getInstance();
ConditionManager.resetInstance();
// Execute the Block
Object result = block.execute(mail);
// Restore the old ConditionManager
ConditionManager.setInstance(oldManager);
return result;
}
I don't get how it works. ConditionManager.resetInstance will set the
static thread local fieldInstance to null: doesn't this invalidate also
instances of every other thread using the ConditionManager ? Shouldn't
resetInstance simply remove the conditionManager from the fieldInstance
ThreadLocal using a fieldInstance.set(null) ?
Stefano
> On 8/6/07, Robert Burrell Donkin <[EMAIL PROTECTED]> wrote:
>> i've been observing sporadic exceptions in my fetchmail->sieve->IMAP.
>> the exceptions are thrown even though the mail filtering works fine
>> when the script is run offline. the majority of these exceptions have
>> been fixed by adding read/write locking to the torque mailbox. so,
>> i've been able to focus on the small number left. these don't appear
>> to be IMAP related.
>>
>> the usual symptom (for these remaining issues) is a failed check in
>> http://svn.apache.org/repos/asf/james/jsieve/trunk/src/main/java/org/apache/jsieve/commands/CommandStateManager.java.
>> i've been wondering recently whether the concurrency isn't quite
>> right in this class. these issues are hard to debug and tend to
>> require a lot of staring at the code. so hopefully, people will help
>> me out by lending an eye ball or two and then stating their opinions.
>>
>> 1 ThreadLocal is created lazily and is used without subclassing.
>> http://java.sun.com/j2se/1.5.0/docs/api/java/lang/ThreadLocal.html
>> recommends subclassing.
>>
>> 2 the instance is set to nulled on a reset. in the case of concurrent
>> threads, a reset in one thread may lead to loss of state in another. a
>> reset on the instance would prevent this.
>>
>> i think that it may better to eliminate the static per-thread
>> singleton manipulation methods and replace them with a ThreadLocal
>> subclass declared final. the reset would then be moved into the
>> instance.
>>
>> opinions?
>>
>> can anyone think of any better approaches?
>>
>> - robert
>>
>> ---------------------------------------------------------------------
>> 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]
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
