Skip to content

Test unlock on exception during scanning #30580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PVince81 merged 4 commits into from
Feb 26, 2018
Merged

Test unlock on exception during scanning #30580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
071b1a3
Test unlock on exception during scanning
DeepDiver1975 Feb 22, 2018
4702e05
Why we should always and everywhere use ::class instead of the explic…
DeepDiver1975 Feb 22, 2018
ff815ab
Add unlock exception test for Scanner::scan
DeepDiver1975 Feb 22, 2018
fac3b8f
Use finally to properly unlock
DeepDiver1975 Feb 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
217 changes: 112 additions & 105 deletions lib/private/Files/Cache/Scanner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,12 @@

use OC\Files\Filesystem;
use OC\Hooks\BasicEmitter;
use OCA\Files_Sharing\ISharedStorage;
use OCP\Config;
use OCP\Files\Cache\IScanner;
use OCP\Files\ForbiddenException;
use OCP\Files\IHomeStorage;
use OCP\Files\Storage\ILockingStorage;
use OCP\Lock\ILockingProvider;

/**
Expand Down Expand Up @@ -107,14 +110,15 @@ public function setUseTransactions($useTransactions) {
*
* @param string $path
* @return array an array of metadata of the file
* @throws \OCP\Files\StorageNotAvailableException
*/
protected function getData($path) {
$data = $this->storage->getMetaData($path);
if (is_null($data)) {
\OCP\Util::writeLog('OC\Files\Cache\Scanner', "!!! Path '$path' is not accessible or present !!!", \OCP\Util::DEBUG);
if ($data === null) {
\OCP\Util::writeLog(Scanner::class, "!!! Path '$path' is not accessible or present !!!", \OCP\Util::DEBUG);
// Last Line of Defence against potential Metadata-loss
if ($this->storage->instanceOfStorage('\OCP\Files\IHomeStorage') && !$this->storage->instanceOfStorage('OCA\Files_Sharing\ISharedStorage') && ($path === '' || $path === 'files')) {
\OCP\Util::writeLog('OC\Files\Cache\Scanner', 'Missing important folder "' . $path . '" in home storage!!! - ' . $this->storageId, \OCP\Util::ERROR);
if ($this->storage->instanceOfStorage(IHomeStorage::class) && !$this->storage->instanceOfStorage(ISharedStorage::class) && ($path === '' || $path === 'files')) {
\OCP\Util::writeLog(Scanner::class, 'Missing important folder "' . $path . '" in home storage!!! - ' . $this->storageId, \OCP\Util::ERROR);
throw new \OCP\Files\StorageNotAvailableException('Missing important folder "' . $path . '" in home storage - ' . $this->storageId);
}
}
Expand All @@ -130,114 +134,111 @@ protected function getData($path) {
* @param array | null $cacheData existing data in the cache for the file to be scanned
* @param bool $lock set to false to disable getting an additional read lock during scanning
* @return array an array of metadata of the scanned file
* @throws \OC\ServerNotAvailableException
* @throws \OCP\Files\StorageNotAvailableException
* @throws \OCP\Lock\LockedException
* @throws \OC\HintException
* @throws \OC\ServerNotAvailableException
*/
public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true) {

// only proceed if $file is not a partial file nor a blacklisted file
if (!self::isPartialFile($file) and !Filesystem::isFileBlacklisted($file)) {

//acquire a lock
if ($lock) {
if ($this->storage->instanceOfStorage('\OCP\Files\Storage\ILockingStorage')) {
$this->storage->acquireLock($file, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
}
if ($lock && $this->storage->instanceOfStorage(ILockingStorage::class)) {
$this->storage->acquireLock($file, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
}

try {
$data = $this->getData($file);
} catch (ForbiddenException $e) {
return null;
}

if ($data) {
if ($data) {

// pre-emit only if it was a file. By that we avoid counting/treating folders as files
if ($data['mimetype'] !== 'httpd/unix-directory') {
$this->emit('\OC\Files\Cache\Scanner', 'scanFile', [$file, $this->storageId]);
\OC_Hook::emit('\OC\Files\Cache\Scanner', 'scan_file', ['path' => $file, 'storage' => $this->storageId]);
}
// pre-emit only if it was a file. By that we avoid counting/treating folders as files
if ($data['mimetype'] !== 'httpd/unix-directory') {
$this->emit('\OC\Files\Cache\Scanner', 'scanFile', [$file, $this->storageId]);
\OC_Hook::emit('\OC\Files\Cache\Scanner', 'scan_file', ['path' => $file, 'storage' => $this->storageId]);
}

$parent = dirname($file);
if ($parent === '.' or $parent === '/') {
$parent = '';
}
if ($parentId === -1) {
$parentId = $this->cache->getId($parent);
}
$parent = dirname($file);
if ($parent === '.' or $parent === '/') {
$parent = '';
}
if ($parentId === -1) {
$parentId = $this->cache->getId($parent);
}

// scan the parent if it's not in the cache (id -1) and the current file is not the root folder
if ($file and $parentId === -1) {
$parentData = $this->scanFile($parent);
$parentId = $parentData['fileid'];
}
if ($parent) {
$data['parent'] = $parentId;
}
if (is_null($cacheData)) {
/** @var CacheEntry $cacheData */
$cacheData = $this->cache->get($file);
}
if ($cacheData and $reuseExisting and isset($cacheData['fileid'])) {
// prevent empty etag
if (empty($cacheData['etag'])) {
$etag = $data['etag'];
} else {
$etag = $cacheData['etag'];
// scan the parent if it's not in the cache (id -1) and the current file is not the root folder
if ($file and $parentId === -1) {
$parentData = $this->scanFile($parent);
$parentId = $parentData['fileid'];
}
$fileId = $cacheData['fileid'];
$data['fileid'] = $fileId;
// only reuse data if the file hasn't explicitly changed
if (isset($data['storage_mtime']) && isset($cacheData['storage_mtime']) && $data['storage_mtime'] === $cacheData['storage_mtime']) {
$data['mtime'] = $cacheData['mtime'];
if (($reuseExisting & self::REUSE_SIZE) && ($data['size'] === -1)) {
$data['size'] = $cacheData['size'];
if ($parent) {
$data['parent'] = $parentId;
}
if (is_null($cacheData)) {
/** @var CacheEntry $cacheData */
$cacheData = $this->cache->get($file);
}
if ($cacheData and $reuseExisting and isset($cacheData['fileid'])) {
// prevent empty etag
if (empty($cacheData['etag'])) {
$etag = $data['etag'];
} else {
$etag = $cacheData['etag'];
}
if ($reuseExisting & self::REUSE_ETAG) {
$data['etag'] = $etag;
$fileId = $cacheData['fileid'];
$data['fileid'] = $fileId;
// only reuse data if the file hasn't explicitly changed
if (isset($data['storage_mtime']) && isset($cacheData['storage_mtime']) && $data['storage_mtime'] === $cacheData['storage_mtime']) {
$data['mtime'] = $cacheData['mtime'];
if (($reuseExisting & self::REUSE_SIZE) && ($data['size'] === -1)) {
$data['size'] = $cacheData['size'];
}
if ($reuseExisting & self::REUSE_ETAG) {
$data['etag'] = $etag;
}
}
// Only update metadata that has changed
$newData = array_diff_assoc($data, $cacheData->getData());
} else {
$newData = $data;
$fileId = -1;
}
if (!empty($newData)) {
$data['fileid'] = $this->addToCache($file, $newData, $fileId);
}
if (isset($cacheData['size'])) {
$data['oldSize'] = $cacheData['size'];
} else {
$data['oldSize'] = 0;
}
// Only update metadata that has changed
$newData = array_diff_assoc($data, $cacheData->getData());
} else {
$newData = $data;
$fileId = -1;
}
if (!empty($newData)) {
$data['fileid'] = $this->addToCache($file, $newData, $fileId);
}
if (isset($cacheData['size'])) {
$data['oldSize'] = $cacheData['size'];
} else {
$data['oldSize'] = 0;
}

if (isset($cacheData['encrypted'])) {
$data['encrypted'] = $cacheData['encrypted'];
}
if (isset($cacheData['encrypted'])) {
$data['encrypted'] = $cacheData['encrypted'];
}

// post-emit only if it was a file. By that we avoid counting/treating folders as files
if ($data['mimetype'] !== 'httpd/unix-directory') {
$this->emit('\OC\Files\Cache\Scanner', 'postScanFile', [$file, $this->storageId]);
\OC_Hook::emit('\OC\Files\Cache\Scanner', 'post_scan_file', ['path' => $file, 'storage' => $this->storageId]);
}
// post-emit only if it was a file. By that we avoid counting/treating folders as files
if ($data['mimetype'] !== 'httpd/unix-directory') {
$this->emit('\OC\Files\Cache\Scanner', 'postScanFile', [$file, $this->storageId]);
\OC_Hook::emit('\OC\Files\Cache\Scanner', 'post_scan_file', ['path' => $file, 'storage' => $this->storageId]);
}

} else {
$this->removeFromCache($file);
}
} else {
$this->removeFromCache($file);
}

//release the acquired lock
if ($lock) {
if ($this->storage->instanceOfStorage('\OCP\Files\Storage\ILockingStorage')) {
if ($data && !isset($data['encrypted'])) {
$data['encrypted'] = false;
}
return $data;
} catch (ForbiddenException $e) {
return null;
} finally {
//release the acquired lock
if ($lock && $this->storage->instanceOfStorage(ILockingStorage::class)) {
$this->storage->releaseLock($file, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
}
}

if ($data && !isset($data['encrypted'])) {
$data['encrypted'] = false;
}
return $data;
}

return null;
Expand All @@ -256,6 +257,8 @@ protected function removeFromCache($path) {
* @param array $data
* @param int $fileId
* @return int the id of the added file
* @throws \OC\HintException
* @throws \OC\ServerNotAvailableException
*/
protected function addToCache($path, $data, $fileId = -1) {
\OC_Hook::emit('Scanner', 'addToCache', ['file' => $path, 'data' => $data]);
Expand All @@ -264,18 +267,20 @@ protected function addToCache($path, $data, $fileId = -1) {
if ($fileId !== -1) {
$this->cache->update($fileId, $data);
return $fileId;
} else {
return $this->cache->put($path, $data);
}
} else {
return -1;

return $this->cache->put($path, $data);
}

return -1;
}

/**
* @param string $path
* @param array $data
* @param int $fileId
* @throws \OC\HintException
* @throws \OC\ServerNotAvailableException
*/
protected function updateCache($path, $data, $fileId = -1) {
\OC_Hook::emit('Scanner', 'addToCache', ['file' => $path, 'data' => $data]);
Expand All @@ -297,24 +302,25 @@ protected function updateCache($path, $data, $fileId = -1) {
* @param int $reuse
* @param bool $lock set to false to disable getting an additional read lock during scanning
* @return array an array of the meta data of the scanned file or folder
* @throws \OCP\Lock\LockedException
* @throws \OC\ServerNotAvailableException
*/
public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $lock = true) {
if ($reuse === -1) {
$reuse = ($recursive === self::SCAN_SHALLOW) ? self::REUSE_ETAG | self::REUSE_SIZE : self::REUSE_ETAG;
}
if ($lock) {
if ($this->storage->instanceOfStorage('\OCP\Files\Storage\ILockingStorage')) {
$this->storage->acquireLock('scanner::' . $path, ILockingProvider::LOCK_EXCLUSIVE, $this->lockingProvider);
$this->storage->acquireLock($path, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
}
}
$data = $this->scanFile($path, $reuse, -1, null, $lock);
if ($data and $data['mimetype'] === 'httpd/unix-directory') {
$size = $this->scanChildren($path, $recursive, $reuse, $data['fileid'], $lock);
$data['size'] = $size;
if ($lock && $this->storage->instanceOfStorage(ILockingStorage::class)) {
$this->storage->acquireLock('scanner::' . $path, ILockingProvider::LOCK_EXCLUSIVE, $this->lockingProvider);
$this->storage->acquireLock($path, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
}
if ($lock) {
if ($this->storage->instanceOfStorage('\OCP\Files\Storage\ILockingStorage')) {
try {
$data = $this->scanFile($path, $reuse, -1, null, $lock);
if ($data and $data['mimetype'] === 'httpd/unix-directory') {
$size = $this->scanChildren($path, $recursive, $reuse, $data['fileid'], $lock);
$data['size'] = $size;
}
} finally {
if ($lock && $this->storage->instanceOfStorage(ILockingStorage::class)) {
$this->storage->releaseLock($path, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
$this->storage->releaseLock('scanner::' . $path, ILockingProvider::LOCK_EXCLUSIVE, $this->lockingProvider);
}
Expand Down Expand Up @@ -366,6 +372,7 @@ protected function getNewChildren($folder) {
* @param int $folderId id for the folder to be scanned
* @param bool $lock set to false to disable getting an additional read lock during scanning
* @return int the size of the scanned folder or -1 if the size is unknown at this stage
* @throws \OCP\Lock\LockedException
*/
protected function scanChildren($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $folderId = null, $lock = true) {
if ($reuse === -1) {
Expand Down Expand Up @@ -405,7 +412,7 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
$exceptionOccurred = false;
$childQueue = [];
foreach ($newChildren as $file) {
$child = ($path) ? $path . '/' . $file : $file;
$child = $path ? $path . '/' . $file : $file;
try {
$existingData = isset($existingChildren[$file]) ? $existingChildren[$file] : null;
$data = $this->scanFile($child, $reuse, $folderId, $existingData, $lock);
Expand All @@ -429,14 +436,14 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
$exceptionOccurred = true;
} catch (\OCP\Lock\LockedException $e) {
if ($this->useTransactions) {
\OC::$server->getDatabaseConnection()->rollback();
\OC::$server->getDatabaseConnection()->rollBack();
}
throw $e;
}
}
$removedChildren = \array_diff(array_keys($existingChildren), $newChildren);
foreach ($removedChildren as $childName) {
$child = ($path) ? $path . '/' . $childName : $childName;
$child = $path ? $path . '/' . $childName : $childName;
$this->removeFromCache($child);
}
if ($this->useTransactions) {
Expand Down
2 changes: 1 addition & 1 deletion lib/private/Files/Storage/Temporary.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
/**
* local storage backend in temporary folder for testing purpose
*/
class Temporary extends Local{
class Temporary extends Local {
public function __construct($arguments = null) {
parent::__construct(['datadir' => \OC::$server->getTempManager()->getTemporaryFolder()]);
}
Expand Down
Loading