shared lock around hooks

This commit is contained in:
Robin Appelman 2015-05-29 16:35:49 +02:00
parent 661c9e2444
commit ce04cf6610
2 changed files with 46 additions and 11 deletions

View File

@ -534,6 +534,7 @@ class View {
and !Filesystem::isFileBlacklisted($path) and !Filesystem::isFileBlacklisted($path)
) { ) {
$path = $this->getRelativePath($absolutePath); $path = $this->getRelativePath($absolutePath);
$exists = $this->file_exists($path); $exists = $this->file_exists($path);
$run = true; $run = true;
if ($this->shouldEmitHooks($path)) { if ($this->shouldEmitHooks($path)) {
@ -613,6 +614,10 @@ class View {
if ($path1 == null or $path2 == null) { if ($path1 == null or $path2 == null) {
return false; return false;
} }
$this->lockFile($path1, ILockingProvider::LOCK_SHARED);
$this->lockFile($path2, ILockingProvider::LOCK_SHARED);
$run = true; $run = true;
if ($this->shouldEmitHooks() && (Cache\Scanner::isPartialFile($path1) && !Cache\Scanner::isPartialFile($path2))) { if ($this->shouldEmitHooks() && (Cache\Scanner::isPartialFile($path1) && !Cache\Scanner::isPartialFile($path2))) {
// if it was a rename from a part file to a regular file it was a write and not a rename operation // if it was a rename from a part file to a regular file it was a write and not a rename operation
@ -638,13 +643,8 @@ class View {
$internalPath1 = $mount1->getInternalPath($absolutePath1); $internalPath1 = $mount1->getInternalPath($absolutePath1);
$internalPath2 = $mount2->getInternalPath($absolutePath2); $internalPath2 = $mount2->getInternalPath($absolutePath2);
$this->lockFile($path1, ILockingProvider::LOCK_EXCLUSIVE); $this->changeLock($path1, ILockingProvider::LOCK_EXCLUSIVE);
try { $this->changeLock($path2, ILockingProvider::LOCK_EXCLUSIVE);
$this->lockFile($path2, ILockingProvider::LOCK_EXCLUSIVE);
} catch (LockedException $e) {
$this->unlockFile($path1, ILockingProvider::LOCK_EXCLUSIVE);
throw $e;
}
if ($internalPath1 === '' and $mount1 instanceof MoveableMount) { if ($internalPath1 === '' and $mount1 instanceof MoveableMount) {
if ($this->isTargetAllowed($absolutePath2)) { if ($this->isTargetAllowed($absolutePath2)) {
@ -702,6 +702,8 @@ class View {
} }
return $result; return $result;
} else { } else {
$this->unlockFile($path1, ILockingProvider::LOCK_SHARED);
$this->unlockFile($path2, ILockingProvider::LOCK_SHARED);
return false; return false;
} }
} else { } else {
@ -733,6 +735,10 @@ class View {
return false; return false;
} }
$run = true; $run = true;
$this->lockFile($path2, ILockingProvider::LOCK_SHARED);
$this->lockFile($path1, ILockingProvider::LOCK_SHARED);
$exists = $this->file_exists($path2); $exists = $this->file_exists($path2);
if ($this->shouldEmitHooks()) { if ($this->shouldEmitHooks()) {
\OC_Hook::emit( \OC_Hook::emit(
@ -754,7 +760,7 @@ class View {
$storage2 = $mount2->getStorage(); $storage2 = $mount2->getStorage();
$internalPath2 = $mount2->getInternalPath($absolutePath2); $internalPath2 = $mount2->getInternalPath($absolutePath2);
$this->lockFile($path2, ILockingProvider::LOCK_EXCLUSIVE); $this->changeLock($path2, ILockingProvider::LOCK_EXCLUSIVE);
if ($mount1->getMountPoint() == $mount2->getMountPoint()) { if ($mount1->getMountPoint() == $mount2->getMountPoint()) {
if ($storage1) { if ($storage1) {
@ -768,6 +774,7 @@ class View {
$this->updater->update($path2); $this->updater->update($path2);
$this->unlockFile($path2, ILockingProvider::LOCK_EXCLUSIVE); $this->unlockFile($path2, ILockingProvider::LOCK_EXCLUSIVE);
$this->unlockFile($path1, ILockingProvider::LOCK_SHARED);
if ($this->shouldEmitHooks() && $result !== false) { if ($this->shouldEmitHooks() && $result !== false) {
\OC_Hook::emit( \OC_Hook::emit(
@ -782,6 +789,8 @@ class View {
} }
return $result; return $result;
} else { } else {
$this->unlockFile($path2, ILockingProvider::LOCK_SHARED);
$this->unlockFile($path1, ILockingProvider::LOCK_SHARED);
return false; return false;
} }
} else { } else {
@ -961,13 +970,16 @@ class View {
return false; return false;
} }
if(in_array('write', $hooks) || in_array('delete', $hooks) || in_array('read', $hooks)) {
// always a shared lock during pre-hooks so the hook can read the file
$this->lockFile($path, ILockingProvider::LOCK_SHARED);
}
$run = $this->runHooks($hooks, $path); $run = $this->runHooks($hooks, $path);
list($storage, $internalPath) = Filesystem::resolvePath($absolutePath . $postFix); list($storage, $internalPath) = Filesystem::resolvePath($absolutePath . $postFix);
if ($run and $storage) { if ($run and $storage) {
if (in_array('write', $hooks) || in_array('delete', $hooks)) { if (in_array('write', $hooks) || in_array('delete', $hooks)) {
$this->lockFile($path, ILockingProvider::LOCK_EXCLUSIVE); $this->changeLock($path, ILockingProvider::LOCK_EXCLUSIVE);
} else if (in_array('read', $hooks)) {
$this->lockFile($path, ILockingProvider::LOCK_SHARED);
} }
try { try {
if (!is_null($extraParam)) { if (!is_null($extraParam)) {
@ -1017,6 +1029,8 @@ class View {
} }
} }
return $result; return $result;
} else {
$this->unlockFile($path, ILockingProvider::LOCK_SHARED);
} }
} }
return null; return null;
@ -1661,6 +1675,23 @@ class View {
} }
} }
/**
* @param string $path the path of the file to lock, relative to the view
* @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE
*/
private function changeLock($path, $type) {
$mount = $this->getMount($path);
if ($mount) {
$mount->getStorage()->changeLock(
$mount->getInternalPath(
$this->getAbsolutePath($path)
),
$type,
$this->lockingProvider
);
}
}
/** /**
* @param string $path the path of the file to unlock, relative to the view * @param string $path the path of the file to unlock, relative to the view
* @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE

View File

@ -110,11 +110,15 @@ class MemcacheLockingProvider implements ILockingProvider {
if (!$this->memcache->cas($path, 'exclusive', 1)) { if (!$this->memcache->cas($path, 'exclusive', 1)) {
throw new LockedException($path); throw new LockedException($path);
} }
unset($this->acquiredLocks['exclusive'][$path]);
$this->acquiredLocks['shared'][$path]++;
} else if ($targetType === self::LOCK_EXCLUSIVE) { } else if ($targetType === self::LOCK_EXCLUSIVE) {
// we can only change a shared lock to an exclusive if there's only a single owner of the shared lock // we can only change a shared lock to an exclusive if there's only a single owner of the shared lock
if (!$this->memcache->cas($path, 1, 'exclusive')) { if (!$this->memcache->cas($path, 1, 'exclusive')) {
throw new LockedException($path); throw new LockedException($path);
} }
$this->acquiredLocks['exclusive'][$path] = true;
$this->acquiredLocks['shared'][$path]--;
} }
} }