{"action":"closed","issue":{"url":"https://api.github.com/repos/owncloud/core/issues/16664","labels_url":"https://api.github.com/repos/owncloud/core/issues/16664/labels{/name}","comments_url":"https://api.github.com/repos/owncloud/core/issues/16664/comments","events_url":"https://api.github.com/repos/owncloud/core/issues/16664/events","html_url":"https://github.com/owncloud/core/issues/16664","id":83550603,"number":16664,"title":"Issues to solve with locking","user":{"login":"PVince81","id":277525,"avatar_url":"https://avatars.githubusercontent.com/u/277525?v=3","gravatar_id":"","url":"https://api.github.com/users/PVince81","html_url":"https://github.com/PVince81","followers_url":"https://api.github.com/users/PVince81/followers","following_url":"https://api.github.com/users/PVince81/following{/other_user}","gists_url":"https://api.github.com/users/PVince81/gists{/gist_id}","starred_url":"https://api.github.com/users/PVince81/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/PVince81/subscriptions","organizations_url":"https://api.github.com/users/PVince81/orgs","repos_url":"https://api.github.com/users/PVince81/repos","events_url":"https://api.github.com/users/PVince81/events{/privacy}","received_events_url":"https://api.github.com/users/PVince81/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/owncloud/core/labels/app:files_versions","name":"app:files_versions","color":"02d7e1"},{"url":"https://api.github.com/repos/owncloud/core/labels/bug","name":"bug","color":"fc2929"},{"url":"https://api.github.com/repos/owncloud/core/labels/feature:locking","name":"feature:locking","color":"d4c5f9"},{"url":"https://api.github.com/repos/owncloud/core/labels/sev2-high","name":"sev2-high","color":"fc2929"}],"state":"closed","locked":false,"assignee":{"login":"PVince81","id":277525,"avatar_url":"https://avatars.githubusercontent.com/u/277525?v=3","gravatar_id":"","url":"https://api.github.com/users/PVince81","html_url":"https://github.com/PVince81","followers_url":"https://api.github.com/users/PVince81/followers","following_url":"https://api.github.com/users/PVince81/following{/other_user}","gists_url":"https://api.github.com/users/PVince81/gists{/gist_id}","starred_url":"https://api.github.com/users/PVince81/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/PVince81/subscriptions","organizations_url":"https://api.github.com/users/PVince81/orgs","repos_url":"https://api.github.com/users/PVince81/repos","events_url":"https://api.github.com/users/PVince81/events{/privacy}","received_events_url":"https://api.github.com/users/PVince81/received_events","type":"User","site_admin":false},"milestone":{"url":"https://api.github.com/repos/owncloud/core/milestones/46","html_url":"https://github.com/owncloud/core/milestones/8.1.1-next-maintenance","labels_url":"https://api.github.com/repos/owncloud/core/milestones/46/labels","id":1167392,"number":46,"title":"8.1.1-next-maintenance","description":"","creator":{"login":"cmonteroluque","id":879976,"avatar_url":"https://avatars.githubusercontent.com/u/879976?v=3","gravatar_id":"","url":"https://api.github.com/users/cmonteroluque","html_url":"https://github.com/cmonteroluque","followers_url":"https://api.github.com/users/cmonteroluque/followers","following_url":"https://api.github.com/users/cmonteroluque/following{/other_user}","gists_url":"https://api.github.com/users/cmonteroluque/gists{/gist_id}","starred_url":"https://api.github.com/users/cmonteroluque/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cmonteroluque/subscriptions","organizations_url":"https://api.github.com/users/cmonteroluque/orgs","repos_url":"https://api.github.com/users/cmonteroluque/repos","events_url":"https://api.github.com/users/cmonteroluque/events{/privacy}","received_events_url":"https://api.github.com/users/cmonteroluque/received_events","type":"User","site_admin":false},"open_issues":14,"closed_issues":1,"state":"open","created_at":"2015-06-16T19:39:31Z","updated_at":"2015-07-01T09:28:09Z","due_on":null,"closed_at":null},"comments":12,"created_at":"2015-06-01T15:07:32Z","updated_at":"2015-07-01T09:28:09Z","closed_at":"2015-07-01T09:28:09Z","body":"- [x] TODO: lock scanner: prevent the scanner to scan or modify/delete locked elements (see https://github.com/owncloud/core/issues/13391#issuecomment-105852842). Maybe the scanner should also use acquireLock to make sure other processes do not try and do something with the cache entries that are about to be changed. This also means that concurrent scanner processes would exclude each other.\r\n- [x] TODO: also acquire shared lock before rename pre-hooks (as we did in basicOperation): https://github.com/owncloud/core/issues/16829\r\n- [ ] TODO: wrap touch operation from upload (chunking mode) in the transaction: https://github.com/owncloud/core/issues/13498 (non-chunking should be fine). This will also need the \"scanner locking\" to avoid the scanner finding the non-touched-file with a different timestamp. => PR here https://github.com/owncloud/core/pull/17104\r\n- [x] TODO: also properly wrap $view->file_put_contents with locks. I see it calls $view->fopen() but the cache update is done after fclose(). Might be safer to also include the cache update and post hooks inside a transaction/lock. https://github.com/owncloud/core/issues/16727\r\n\r\n- [x] BUG: rename folder during upload still has inconsistency between cache and on-disk data (https://github.com/owncloud/core/issues/11795)\r\n\r\n0. Add `sleep(10);` here: https://github.com/owncloud/core/blob/master/lib/private/files/cache/cache.php#L551\r\n1. Setup sync client\r\n2. Close sync client\r\n3. Copy a big folder \"test\" locally\r\n4. Start the sync client\r\n5. Quickly, while it starts uploading: refresh the web UI\r\n6. Rename \"test\" to \"re\"\r\n7. Sync client fails with error \"could not find folder 'test'\" after a bunch of uploads, which is expected\r\n8. In web UI: \"re\" is still empty, but \"data/$user/re\" contains files\r\n9. Sync client resumes upload, but in the end not all files are present in \"test\"\r\n\r\nWe need to tweak the locking to be able to avoid such situations.\r\n\r\n- [x] rename folder with slow cache update still loses contents, see https://github.com/owncloud/core/issues/13391#issuecomment-105525369\r\n- [x] BUG: properly handle LockedException in web UI: when renaming a locked folder in the web UI: exception returned as \"500 Internal Server Error\" but logged properly:\r\n- [x] BUG: make etag precondition + final rename atomic: https://github.com/owncloud/core/issues/16569 (the current locking did not help, might need to retry after acquireLock is set for small files)\r\n- [x] BUG: sleep after chunk assembly but before final rename, then rename parent folder: sync fails with \"could not rename part file to final file\". We should bundle chunking inside a transaction. But the current behavior might be ok as it doesn't cause any data corruption.\r\n- [x] BUG: [minor] move big folder to ext storage in web UI, sync client running: sync client cannot download any of the moved files as long as the move is happening (even if the files are there already). Once the move is done, the sync client retries and can get the file. Would still be nice to have to be able to download files as soon as they are moved (but keep the lock on the folder)\r\nTo clarify: download should be forbidden in the **source** folder, but not in the **target** folder.\r\n\r\nTests:\r\n\r\n- [X] TEST: move big folder to ext storage. During the slow move, cannot rename source or target folder: locked.\r\n- [x] TEST: cannot rename folder while it is being moved to trash (tested with slow ext storage, copy from storage to trash)\r\n- [x] TEST: cannot upload into a folder while it is being moved to trash\r\n- [x] TEST: cannot rename a folder while chunks are being assembled (added sleep() in the chunk assembly loop), LockedException\r\n- [x] TEST: slow down streamCopy for WebDAV upload and try renaming the parent folder\r\n- [X] TEST: delete folder while uploading inside (with slow streamCopy): denied by locked error\r\n- [X] TEST: kill sync client during upload (with slow streamCopy): lock expires after PHP timeout\r\n- [X] TEST: simulate \"expected filesize did not match\" error: lock was properly released despite exception (shutdown handler)\r\n- [x] TEST: cannot rename target folder when restoring it from trash\r\n- [x] TEST: cannot download files inside a folder that is being moved to trash (source folder)\r\n- [x] TEST: restore big folder from trash to SFTP (slow), try restoring it at the same time. Restore fails due to lock.\r\n- [x] TEST: rename \"test\" to \"test2\", sleep before the actual rename: cannot create \"test2\" in another thread because locked\r\n\r\n#### Sharing (locks must work across users):\r\n- [x] TEST: slow upload of small file as owner, recipient cannot download while locked\r\n- [x] TEST: slow upload of small file as recipient, owner cannot download while locked\r\n- [x] TEST: owner moves folder, recipient cannot rename source or target folder\r\n- [x] TEST: recipient moves folder, owner cannot rename source or target folder\r\n- [x] TEST: delete subfolder as recipient, owner cannot rename/download contents\r\n- [x] TEST: delete subfolder as owner, recipient cannot rename/download contents\r\n- [ ] TEST: owner unshares folder (can be slow with encryption), recipient cannot rename/download folder contents => raised as https://github.com/owncloud/core/issues/17244\r\n- [ ] TEST: delete file in thread A, try and share it in thread B. Share must fail because of lock. :no_entry_sign: => raised as https://github.com/owncloud/core/issues/17243\r\n- [x] TEST: delete subfolder as owner in thread A, recipent can still unshare from self in thread B(TBC?) :question: \r\n- [x] TEST: delete subfolder as recipient in thread A, owner can still unshare from that recipient in thread B (TBC?) :question: works but produces warning (sleep in move2trash after setupTrash)\r\n- [x] TEST: unshare from self twice\r\n- [ ] TEST: share with group/user with encryption enabled (make key creation slow with sleep): recipient should not get access to the files until this is done :no_entry_sign: FAIL, raised as https://github.com/owncloud/core/issues/16621\r\n- [x] TEST: as owner, rename parent folder of shared folder when locked by recipient\r\n- [x] TEST: as recipient, rename parent folder of shared folder when locked by owner\r\n\r\n#### Versions:\r\n- [ ] TEST: cannot download version of a file being moved (neither from source nor target, single and cross-storage) :no_entry_sign: FAIL, missing lock (sleep after acquireLock in $view->rename()) => raised as https://github.com/owncloud/core/issues/17245\r\n- [ ] TEST: cannot restore version of a file being moved (neither from source nor target, single and cross-storage). :no_entry_sign: FAIL: restore is blocked by write lock, but the original file gets deleted! (sleep after acquireLock in $view->rename()) => https://github.com/owncloud/core/issues/17245\r\n- [ ] TEST: restore version (with sleep in restore code) in thread A, cannot rename or overwrite the file in thread B => https://github.com/owncloud/core/issues/17269\r\n- [x] TEST: sleep in loop that moves each version (https://github.com/owncloud/core/blob/master/apps/files_versions/lib/storage.php#L270); thread A moves file, thread B overwrites new file name. While the versions are moved (post hook), the file is still locked with a share lock: https://github.com/owncloud/core/pull/17030\r\n- [ ] TEST: sleep in version expiration loop (https://github.com/owncloud/core/blob/master/apps/files_versions/lib/storage.php#L617), trigger cron.php in thread A, try moving the file in thread B :no_entry_sign:\r\n\r\n#### Previews:\r\n- [x] TEST: if file was locked (not downloadable) during preview creation, preview needs to retry later\r\n(achieved by overwriting a pic file that has no preview yet, and sleep in streamCopy)\r\n\r\n#### Encryption:\r\n- [x] TEST: general testing if locking works like\r\n\r\n#### Zip download:\r\n- [x] TEST: download zip of a folder containing a locked new file (sleep after acquireLock before final rename) => new file does not appear in zip\r\n- [ ] TEST: download zip of a folder containing a locked file to be overwritten (sleep after acquireLock before final rename) => zip file is empty :no_entry_sign: FAIL, should skip locked file ?! => https://github.com/owncloud/core/issues/16960\r\n\r\n#### s2s and locking\r\n- [ ] TEST: share \"local\" from server A to server B. On server A, slow-upload+overwrite file (sleep after acquireLock in sabre/file). Server B cannot download. :no_entry_sign: FAIL for some reason server B can still download => raised as https://github.com/owncloud/core/issues/17275"}}