tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ohad Shacham <ohad.shac...@cs.tau.ac.il>
Subject Re: Atomicity violation in removeAttribute
Date Mon, 27 Sep 2010 20:34:10 GMT
> 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 <sebbaz@gmail.com> wrote:

> On 26 September 2010 15:00, Ohad Shacham <ohad.shacham@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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message