> If get() cannot return null, then I think one can just use:
> value = attributes.remove(name);
> if (value == null)
> return;
> If it is important to distinguish null entries from missing entries,
> then a different approach is needed - e.g. AtomicReference as
> suggested elsewhere
ConcurrentHashMap does not support null keys and entries.
I would use:
if (attributes.containsKey(name)) {
value = attributes.remove(name);
if (value == null)
return;
}
else {
return;
}
because containsKey does not acquire a lock while remove does.
Could you please let me know whether you consider this as a bug?
Thx,
Ohad
On Mon, Sep 27, 2010 at 12:04 PM, sebb <seb...@gmail.com> wrote:
> On 26 September 2010 15:00, Ohad Shacham <ohad.shac...@gmail.com> wrote:
> > Hi,
> >
> >
> > In addition to the behavior we experienced in function setAttribute. We
> also
> > experienced another atomicity violation in function removeAttribute.
> >
> > The following code is located at the beginning of function
> removeAttribute
> > at class ApplicationContext. This code fragment intends to check whether
> a
> > key is already inside the “attributes” collection. If it does, then the
> key
> > is removed from the collection and the function continues running while
> > assigning the corresponding value to “value”. Otherwise, the function
> should
> > return.
> >
> >
> >
> >
> > found = attributes.containsKey(name);
> >
> > if (found) {
> >
> > value = attributes.get(name);
> >
> > attributes.remove(name);
> >
> > } else {
> >
> > return;
> >
> > }
> >
> >
> >
> > This code can behave non atomically when two different threads call
> > removeAttribute function with the same attribute name. In this case,
> there
> > exists a thread interleaving where two threads enter the “if” condition
> > while the first one removes “name” from the attributes collection before
> the
> > second one runs the get operation. This causes the function to continue
> > running with value == null instead of returning.
> >
> >
> >
> > Could you please let me know whether you consider this as a bug?
> >
> >
> >
> > A possible way of modifying the code in order to execute this code
> > atomically is as follows:
> >
> >
> >
> > if (attributes.containsKey(name)) {
> > value = attributes.remove(name);
> > if (value == null)
> > return;
> > }
>
> That's not quite the same if atttributes.get() can return null, as
> previously the null would be used in subsequent code.
>
> If get() cannot return null, then I think one can just use:
>
> value = attributes.remove(name);
> if (value == null)
> return;
>
> If it is important to distinguish null entries from missing entries,
> then a different approach is needed - e.g. AtomicReference as
> suggested elsewhere
>
> >
> >
> > Thanks,
> >
> > Ohad
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>
