On 8/9/07, Stefano Bagnara <[EMAIL PROTECTED]> wrote:
> Bernd Fondermann ha scritto:
> > What types of exceptions do you get? NPE?
mostly ones related to the state machine. it's the same script and it
filters the mail fine in a single threaded test environment but
occasionally fails when run from the spool.
> > 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!!
+1
probably better to replace lazy construction with a final variable
> > 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 ?
good spot 8-)
+1
probably better to use a stack and push the implementation into ConditionManager
> Shouldn't
> resetInstance simply remove the conditionManager from the fieldInstance
> ThreadLocal using a fieldInstance.set(null) ?
i've been running with this change in for a couple of days and (as
yet) there haven't been any exceptions thrown
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
