[jboss-dev-forums] [JBoss Microcontainer Development] - MemoryMetaDataLoader not threadsafe?

[kabir.khan@jboss.com]

Although it will not happen when describing this metadata class, the following does not look very thread safe to me.


  |    @Override
  |    public void describeVisit(MetaDataVisitor vistor)
  |    {
  |       super.describeVisit(vistor);
  |       KernelControllerContext context = vistor.getControllerContext();
  |       MetaDataRetrieval retrieval = context.getKernel().getMetaDataRepository().getMetaDataRepository().getMetaDataRetrieval(context.getScopeInfo().getMutableScope());
  |       if (retrieval instanceof MutableMetaData)
  |       {
  |          MetaDataItem<?> item = retrieval.retrieveMetaData(QUALIFIER_KEY);
  |          //TODO - The following is not threadsafe
  |          List<Object> list = null;
  |          if (item == null)
  |          {
  |             list = new CopyOnWriteArrayList<Object>();
  |             ((MutableMetaData)retrieval).addMetaData(QUALIFIER_KEY, list, List.class);
  |          }
  |          else
  |          {
  |             list = (List<Object>)item.getValue();
  |          }
  |          list.addAll(getEnabled());
  |       }
  |    }
  | 

Since MDR is considered for general usage, it might be an issue if two different threads need to do something similar. i.e. check for existance of metadata then add metadata. addMetaData() seems to overwrite what is there so a check is needed, unless ConcurrentMap.putIfAbsent () semantics are added. For simple values this will probably be fine, last one wins and overwrites, but for things like collections we should have the possiblity to merge the new collection and the exisiting collection, if that is what is intended.

View the original post : http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4266800#4266800

Reply to the post : http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&p=4266800