Author: bleny Date: 2010-06-03 16:35:49 +0200 (Thu, 03 Jun 2010) New Revision: 59 Url: http://nuiton.org/repositories/revision/diswork/59 Log: concurrency, - memory leak Modified: trunk/diswork-fs/src/main/java/org/nuiton/disworkfs/DisworkFileSystem.java trunk/diswork-fs/src/main/java/org/nuiton/disworkfs/storage/Storage.java trunk/diswork-fs/src/test/java/org/nuiton/disworkfs/DisworkFileSystemTest.java Modified: trunk/diswork-fs/src/main/java/org/nuiton/disworkfs/DisworkFileSystem.java =================================================================== --- trunk/diswork-fs/src/main/java/org/nuiton/disworkfs/DisworkFileSystem.java 2010-06-03 13:11:32 UTC (rev 58) +++ trunk/diswork-fs/src/main/java/org/nuiton/disworkfs/DisworkFileSystem.java 2010-06-03 14:35:49 UTC (rev 59) @@ -137,9 +137,9 @@ if (EntryUtil.isDirectory(entryParent)) { String parentId = EntryUtil.getIdFromEntry(entryParent); + + // checking that file do not already exists in this directory String content = storage.getDirectory(parentId); - - // checking that file do not already exists in this directory String findResult = EntryUtil.findEntryInDirectory (content, fileName); if (findResult != null) { @@ -147,11 +147,8 @@ (parent + " already contains an element named " + fileName); } + // file do not exists, write file on the FS String newFileId = EntryUtil.generateId(); - String newContent = EntryUtil.addEntryToDirectoryContent( - content, EntryUtil.TYPE.F, fileName, newFileId); - - // store file before meta info storage.putFile(newFileId, source); // update directory content @@ -160,6 +157,10 @@ while (numberOfTry <= LOCK_MAX_NUMBER_OF_TRY && !lockAcquired) { lockAcquired = storage.tryToLock(parentId); if (lockAcquired) { + // parent dir is locked, do the update + content = storage.getDirectory(parentId); + String newContent = EntryUtil.addEntryToDirectoryContent( + content, EntryUtil.TYPE.F, fileName, newFileId); storage.putDirectory(parentId, newContent); storage.unLock(parentId); } else { @@ -167,7 +168,7 @@ try { Thread.sleep(LOCK_WAIT); } catch (InterruptedException e) { - log.info("exception catch", e); + log.info("wait for lock interrupted", e); throw new IOException ("interrupted while trying to acquire lock", e); } @@ -219,16 +220,49 @@ if (EntryUtil.isDirectory(entryParent)) { String parentId = EntryUtil.getIdFromEntry(entryParent); String content = storage.getDirectory(parentId); + + // first, check if nothing already exists with this name + String findResult = EntryUtil.findEntryInDirectory + (content, dirName); + if (findResult != null) { + throw new IOException + (parent + " already contains an element named " + dirName); + } + + // store file before meta info String newDirectoryId = EntryUtil.generateId(); - String newContent = EntryUtil.addEntryToDirectoryContent( - content, EntryUtil.TYPE.D, dirName, newDirectoryId); - - // store file before meta info storage.putDirectory(newDirectoryId, EntryUtil.EMPTY_DIRECTORY_CONTENT); - // update meta info directory - storage.putDirectory(parentId, newContent); + // update directory content + int numberOfTry = 0; + boolean lockAcquired = false; + while (numberOfTry <= LOCK_MAX_NUMBER_OF_TRY && !lockAcquired) { + lockAcquired = storage.tryToLock(parentId); + if (lockAcquired) { + // we have locked, do the update + content = storage.getDirectory(parentId); + String newContent = EntryUtil.addEntryToDirectoryContent( + content, EntryUtil.TYPE.D, dirName, newDirectoryId); + storage.putDirectory(parentId, newContent); + storage.unLock(parentId); + } else { + log.info(parent + " is locked and can't be written"); + try { + Thread.sleep(LOCK_WAIT); + } catch (InterruptedException e) { + log.info("wait for lock interrupted", e); + throw new IOException + ("interrupted while trying to acquire lock", e); + } + } + } + + if (!lockAcquired) { + // fail, parent dir have not been written + throw new ConcurrentModificationException + ("can't write " + parent + " directory is locked"); + } } else if (EntryUtil.isLink(entryParent)) { String linkTarget = storage.getLink( EntryUtil.getIdFromEntry(entryParent)); @@ -282,19 +316,55 @@ } if (EntryUtil.isDirectory(entryParent)) { + String parentId = EntryUtil.getIdFromEntry(entryParent); String content = storage.getDirectory(parentId); + + // first, check if nothing already exists with this name + String findResult = EntryUtil.findEntryInDirectory + (content, name); + if (findResult != null) { + throw new IOException + (parent + " already contains an element named " + name); + } + + // store file before meta info String newLinkId = EntryUtil.generateId(); - String newContent = EntryUtil.addEntryToDirectoryContent( - content, EntryUtil.TYPE.L, name, newLinkId); + storage.putLink(newLinkId, target); - // store link before meta info - storage.putLink(newLinkId, target); - // update meta info directory - storage.putDirectory(parentId, newContent); + // update directory content + int numberOfTry = 0; + boolean lockAcquired = false; + while (numberOfTry <= LOCK_MAX_NUMBER_OF_TRY && !lockAcquired) { + lockAcquired = storage.tryToLock(parentId); + if (lockAcquired) { + // we have locked, do the update + content = storage.getDirectory(parentId); + String newContent = EntryUtil.addEntryToDirectoryContent( + content, EntryUtil.TYPE.L, name, newLinkId); + storage.putDirectory(parentId, newContent); + storage.unLock(parentId); + } else { + log.info(parent + " is locked and can't be written"); + try { + Thread.sleep(LOCK_WAIT); + } catch (InterruptedException e) { + log.info("wait for lock interrupted", e); + throw new IOException + ("interrupted while trying to acquire lock", e); + } + } + } + + if (!lockAcquired) { + // fail, parent dir have not been written + throw new ConcurrentModificationException + ("can't write " + parent + " directory is locked"); + } + } else if (EntryUtil.isLink(entryParent)) { String linkTarget = storage.getLink( - EntryUtil.getIdFromEntry(entryParent)); + EntryUtil.getIdFromEntry(entryParent)); String newTarget = EntryUtil.resolveLink(parent, linkTarget); createSymbolicLink(newTarget, name, target); } else if (EntryUtil.isFile(entryParent)) { @@ -328,9 +398,6 @@ * @see #delete(String) */ protected void delete(String parent, String name) throws IOException { - // FIXME 20100602 bleny this implementation leaks - // only the association path => id of the metablock is removed, all - // block remains String entryParent = walk(parent); if (entryParent == null) { @@ -344,10 +411,9 @@ String entry = EntryUtil.findEntryInDirectory(content, name); String idToRemove = EntryUtil.getIdFromEntry(entry); - String newContent = EntryUtil.removeEntryFromEntries(content, name); - - // check if not removing a non-empty directory + // according to the type of the entry name, delete it if (EntryUtil.isDirectory(entry)) { + // check if not removing a non-empty directory String innerDirectoryId = EntryUtil.getIdFromEntry(entry); String innerDirectoryContent = storage.getDirectory(innerDirectoryId); @@ -358,13 +424,49 @@ throw new IOException ("trying to remove a non-empty directory"); } + + // remove it + storage.removeDirectory(idToRemove); + + } else if (EntryUtil.isFile(entry)) { + storage.removeFile(idToRemove); + } else if (EntryUtil.isLink(entry)) { + storage.removeLink(idToRemove); + } else { + log.warn("strange entry" + entryParent); + throw new IOException("strange entry" + entryParent); } - // update meta info directory - storage.putDirectory(parentId, newContent); - // store file before meta info - storage.remove(idToRemove); + // update directory content + int numberOfTry = 0; + boolean lockAcquired = false; + while (numberOfTry <= LOCK_MAX_NUMBER_OF_TRY && !lockAcquired) { + lockAcquired = storage.tryToLock(parentId); + if (lockAcquired) { + // we have locked, do the update + content = storage.getDirectory(parentId); + String newContent = EntryUtil.removeEntryFromEntries(content, name); + storage.putDirectory(parentId, newContent); + storage.unLock(parentId); + } else { + log.info(parent + " is locked and can't be written"); + try { + Thread.sleep(LOCK_WAIT); + } catch (InterruptedException e) { + log.info("wait for lock interrupted", e); + throw new IOException + ("interrupted while trying to acquire lock", e); + } + } + } + + if (!lockAcquired) { + // fail, parent dir have not been written + throw new ConcurrentModificationException + ("can't write " + parent + " directory is locked"); + } + } else if (EntryUtil.isLink(entryParent)) { String linkTarget = storage.getLink( EntryUtil.getIdFromEntry(entryParent)); Modified: trunk/diswork-fs/src/main/java/org/nuiton/disworkfs/storage/Storage.java =================================================================== --- trunk/diswork-fs/src/main/java/org/nuiton/disworkfs/storage/Storage.java 2010-06-03 13:11:32 UTC (rev 58) +++ trunk/diswork-fs/src/main/java/org/nuiton/disworkfs/storage/Storage.java 2010-06-03 14:35:49 UTC (rev 59) @@ -9,8 +9,6 @@ import java.util.List; import java.util.Map; -import javax.swing.RowFilter.Entry; - import org.apache.commons.io.IOUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -196,7 +194,22 @@ map.put(id, contentAsBytes); } + public void removeDirectory(String id) throws IOException { + log.debug("removeDirectory(\"" + id + "\")"); + removeKey(id); + } + public void removeFile(String id) throws IOException { + log.debug("removeFile(\"" + id + "\")"); + removeKey(id); + } + + public void removeLink(String id) { + log.debug("removeLink(\"" + id + "\")"); + map.remove(id); + } + + /** * see {@link #get(String)} */ @@ -213,12 +226,9 @@ * @param value * @throws IOException */ - protected void put(String key, InputStream value) + protected void put(String key, InputStream value) throws IOException, ConcurrentModificationException { - // TODO 20100602 bleny locks should be signed to prevent concurrency : - // due to latency, a lock claimed may be obtained even if someone tryed - // to obtain it before boolean lockAcquired = tryToLock(key); if (lockAcquired) { @@ -269,8 +279,11 @@ } eraseDependenciesOfMetablock(key); - map.put(key, map.get(newDataKey)); + if (isLockStillOwned(key)) { + map.put(key, map.get(newDataKey)); + } + unLock(key); } else { @@ -279,12 +292,18 @@ } + protected void remove(String key) throws IOException, + ConcurrentModificationException { + eraseDependenciesOfMetablock(key); + removeKey(key); + } + /** * due to concurrency, key should not be removed immediately. Someone may * read it now. It will be removed later, using {@link #clean()}. * @param key the key to be removed from the map */ - public void remove(String key) { + public void removeKey(String key) { idsToBeRemoved.add(key); } @@ -314,7 +333,7 @@ log.info("trying to acquire a lock on " + key); String lockKey = keyToLockKey(key); - byte[] lock = map.put(lockKey, EntryUtil.newLock(ownerId)); + byte[] lock = map.put(lockKey, EntryUtil.newLock(ownerId)); Boolean result = null; // if there was no lock or if current lock is mine @@ -356,7 +375,7 @@ * the value at this key has to be a metablock * @param key */ - public void eraseDependenciesOfMetablock(String key) { + protected void eraseDependenciesOfMetablock(String key) { byte[] value = map.get(key); if (value != null) { String obsoleteMetaBlock = EntryUtil.bytesToString(value); @@ -365,11 +384,19 @@ EntryUtil.getBlockIdsFromMetaBlock(obsoleteMetaBlock); log.info("removing " + obsoleteBlocksIds.length + " old blocks"); for (String obsoleteBlockId : obsoleteBlocksIds) { - map.remove(obsoleteBlockId); + removeKey(obsoleteBlockId); } } } } + + private boolean isLockStillOwned(String key) { + String lockKey = keyToLockKey(key); + byte[] lock = map.get(lockKey); + String lockOwner = EntryUtil.getOwnerFromLock(lock); + boolean isLockOwned = ownerId.equals(lockOwner); + return isLockOwned; + } protected boolean isObsolete(byte[] lock) { Long currentDate = System.currentTimeMillis(); Modified: trunk/diswork-fs/src/test/java/org/nuiton/disworkfs/DisworkFileSystemTest.java =================================================================== --- trunk/diswork-fs/src/test/java/org/nuiton/disworkfs/DisworkFileSystemTest.java 2010-06-03 13:11:32 UTC (rev 58) +++ trunk/diswork-fs/src/test/java/org/nuiton/disworkfs/DisworkFileSystemTest.java 2010-06-03 14:35:49 UTC (rev 59) @@ -341,6 +341,6 @@ readResult = fileSystem.readDirectory("/otherdir"); assertEquals(1, readResult.size()); assertTrue(readResult.contains("finaldir")); - + } }