Merge pull request #170 from spdx/issue167

Make ModelObject threadsafe on create and getObjectPropertyValue
spdx · Apr 24, 2023 · f2086d2 · f2086d2
2 parents da021df + 098cdcc
commit f2086d2
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 16 deletions.
diff --git a/src/main/java/org/spdx/library/ModelCopyManager.java b/src/main/java/org/spdx/library/ModelCopyManager.java
@@ -29,6 +29,7 @@
 import org.spdx.library.model.SimpleUriValue;
 import org.spdx.library.model.TypedValue;
 import org.spdx.storage.IModelStore;
+import org.spdx.storage.IModelStore.IModelStoreLock;
 import org.spdx.storage.IModelStore.IdType;
 
 /**
@@ -196,10 +197,17 @@ public void copy(IModelStore toStore, String toDocumentUri, String toId,
 	 */
 	private void copyIndividualProperty(IModelStore toStore, String toDocumentUri, String toId, IModelStore fromStore,
             String fromDocumentUri, String fromId, String propName, boolean excludeLicenseDetails) throws InvalidSPDXAnalysisException {
-       if (fromStore.isCollectionProperty(fromDocumentUri, fromId, propName)) {
-            throw new InvalidSPDXAnalysisException("Property "+propName+" is a collection type");
-        }
-       Optional<Object> result =  fromStore.getValue(fromDocumentUri, fromId, propName);
+		IModelStoreLock fromStoreLock = fromStore.enterCriticalSection(fromDocumentUri, false);
+		//Note: we use a write lock since the RDF store may end up creating a property to check if it is a collection
+		Optional<Object> result = Optional.empty();
+		try {
+			if (fromStore.isCollectionProperty(fromDocumentUri, fromId, propName)) {
+	            throw new InvalidSPDXAnalysisException("Property "+propName+" is a collection type");
+	        }
+			result =  fromStore.getValue(fromDocumentUri, fromId, propName);
+		} finally {
+			fromStoreLock.unlock();
+		}
         if (result.isPresent()) {
             if (result.get() instanceof IndividualUriValue) {
                 toStore.setValue(toDocumentUri, toId, propName, new SimpleUriValue((IndividualUriValue)result.get()));
@@ -232,10 +240,17 @@ private void copyIndividualProperty(IModelStore toStore, String toDocumentUri, S
 	 */
 	private void copyCollectionProperty(IModelStore toStore, String toDocumentUri, String toId, IModelStore fromStore,
             String fromDocumentUri, String fromId, String propName, boolean excludeLicenseDetails) throws InvalidSPDXAnalysisException {
-	    if (!fromStore.isCollectionProperty(fromDocumentUri, fromId, propName)) {
-	        throw new InvalidSPDXAnalysisException("Property "+propName+" is not a collection type");
-	    }
-	    Iterator<Object> fromListIter = fromStore.listValues(fromDocumentUri, fromId, propName);
+		IModelStoreLock fromStoreLock = fromStore.enterCriticalSection(fromDocumentUri, false);
+		//Note: we use a write lock since the RDF store may end up creating a property to check if it is a collection
+		Iterator<Object> fromListIter = null;
+		try {
+			if (!fromStore.isCollectionProperty(fromDocumentUri, fromId, propName)) {
+		        throw new InvalidSPDXAnalysisException("Property "+propName+" is not a collection type");
+		    }
+		    fromListIter = fromStore.listValues(fromDocumentUri, fromId, propName);
+		} finally {
+			fromStoreLock.unlock();
+		}
         while (fromListIter.hasNext()) {
             Object listItem = fromListIter.next();
             Object toStoreItem;

diff --git a/src/main/java/org/spdx/library/model/ModelObject.java b/src/main/java/org/spdx/library/model/ModelObject.java
@@ -201,7 +201,15 @@ public ModelObject(IModelStore modelStore, String documentUri, String id, @Nulla
 			}
 		} else {
 			if (create) {
-				modelStore.create(documentUri, id, getType());
+				IModelStoreLock lock = enterCriticalSection(false);
+				// re-check since previous check was done outside of the lock
+				try {
+					if (!modelStore.exists(documentUri, id)) {
+						modelStore.create(documentUri, id, getType());
+					}
+				} finally {
+					lock.unlock();
+				}
 			} else {
 				throw new SpdxIdNotFoundException(id+" does not exist in document "+documentUri);
 			}
@@ -337,13 +345,19 @@ protected Optional<Object> getObjectPropertyValue(String propertyName) throws In
 	 */
 	protected static Optional<Object> getObjectPropertyValue(IModelStore stModelStore, String stDocumentUri,
 			String stId, String propertyName, ModelCopyManager copyManager) throws InvalidSPDXAnalysisException {
-		if (!stModelStore.exists(stDocumentUri, stId)) {
-			return Optional.empty();
-		} else if (stModelStore.isCollectionProperty(stDocumentUri, stId, propertyName)) {
-			return Optional.of(new ModelCollection<>(stModelStore, stDocumentUri, stId, propertyName, copyManager, null));
-		} else {
-			return ModelStorageClassConverter.optionalStoredObjectToModelObject(stModelStore.getValue(stDocumentUri, stId, propertyName), 
-					stDocumentUri, stModelStore, copyManager);
+		IModelStoreLock lock = stModelStore.enterCriticalSection(stDocumentUri, false);
+		// NOTE: we use a write lock since the ModelStorageClassConverter may end up creating objects in the store
+		try {
+			if (!stModelStore.exists(stDocumentUri, stId)) {
+				return Optional.empty();
+			} else if (stModelStore.isCollectionProperty(stDocumentUri, stId, propertyName)) {
+				return Optional.of(new ModelCollection<>(stModelStore, stDocumentUri, stId, propertyName, copyManager, null));
+			} else {
+				return ModelStorageClassConverter.optionalStoredObjectToModelObject(stModelStore.getValue(stDocumentUri, stId, propertyName), 
+						stDocumentUri, stModelStore, copyManager);
+			}
+		} finally {
+			lock.unlock();
 		}
 	}