3 Replies Latest reply on Nov 20, 2009 11:56 AM by kabirkhan

    MemoryMetaDataLoader not threadsafe?

    kabirkhan

      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.