From da2d4250446b6e2f921bd3de0420738881792bd3 Mon Sep 17 00:00:00 2001
From: Robin Appelman <robin@icewind.nl>
Date: Tue, 30 Jun 2020 18:10:42 +0200
Subject: [PATCH] add proper paths to locking exceptions

while some code paths do wrap the "raw" locking exception into one with a proper path, not all of them do
by adding the proper path to the original exception we ensure that we always have the usefull information in out logs

Signed-off-by: Robin Appelman <robin@icewind.nl>
---
 lib/private/Files/Storage/Common.php         | 2 +-
 lib/private/Lock/DBLockingProvider.php       | 4 ++--
 lib/private/Lock/MemcacheLockingProvider.php | 7 ++++---
 lib/private/Lock/NoopLockingProvider.php     | 2 +-
 lib/public/Lock/ILockingProvider.php         | 3 ++-
 lib/public/Lock/LockedException.php          | 9 +++++++--
 6 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/lib/private/Files/Storage/Common.php b/lib/private/Files/Storage/Common.php
index 001b16cb3b..6c57405619 100644
--- a/lib/private/Files/Storage/Common.php
+++ b/lib/private/Files/Storage/Common.php
@@ -747,7 +747,7 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage {
 			);
 		}
 		try {
-			$provider->acquireLock('files/' . md5($this->getId() . '::' . trim($path, '/')), $type);
+			$provider->acquireLock('files/' . md5($this->getId() . '::' . trim($path, '/')), $type, $this->getId() . '::' . $path);
 		} catch (LockedException $e) {
 			if ($logger) {
 				$logger->logException($e, ['level' => ILogger::INFO]);
diff --git a/lib/private/Lock/DBLockingProvider.php b/lib/private/Lock/DBLockingProvider.php
index c32d7b4877..f48bf57028 100644
--- a/lib/private/Lock/DBLockingProvider.php
+++ b/lib/private/Lock/DBLockingProvider.php
@@ -180,7 +180,7 @@ class DBLockingProvider extends AbstractLockingProvider {
 	 * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE
 	 * @throws \OCP\Lock\LockedException
 	 */
-	public function acquireLock(string $path, int $type) {
+	public function acquireLock(string $path, int $type, string $readablePath = null) {
 		$expire = $this->getExpireTime();
 		if ($type === self::LOCK_SHARED) {
 			if (!$this->isLocallyLocked($path)) {
@@ -208,7 +208,7 @@ class DBLockingProvider extends AbstractLockingProvider {
 			}
 		}
 		if ($result !== 1) {
-			throw new LockedException($path);
+			throw new LockedException($path, null, null, $readablePath);
 		}
 		$this->markAcquire($path, $type);
 	}
diff --git a/lib/private/Lock/MemcacheLockingProvider.php b/lib/private/Lock/MemcacheLockingProvider.php
index 3d7510f43f..c9dfcea319 100644
--- a/lib/private/Lock/MemcacheLockingProvider.php
+++ b/lib/private/Lock/MemcacheLockingProvider.php
@@ -71,17 +71,18 @@ class MemcacheLockingProvider extends AbstractLockingProvider {
 	/**
 	 * @param string $path
 	 * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE
+	 * @param string $readablePath human readable path to use in error messages
 	 * @throws \OCP\Lock\LockedException
 	 */
-	public function acquireLock(string $path, int $type) {
+	public function acquireLock(string $path, int $type, string $readablePath = null) {
 		if ($type === self::LOCK_SHARED) {
 			if (!$this->memcache->inc($path)) {
-				throw new LockedException($path, null, $this->getExistingLockForException($path));
+				throw new LockedException($path, null, $this->getExistingLockForException($path), $readablePath);
 			}
 		} else {
 			$this->memcache->add($path, 0);
 			if (!$this->memcache->cas($path, 0, 'exclusive')) {
-				throw new LockedException($path, null, $this->getExistingLockForException($path));
+				throw new LockedException($path, null, $this->getExistingLockForException($path), $readablePath);
 			}
 		}
 		$this->setTTL($path);
diff --git a/lib/private/Lock/NoopLockingProvider.php b/lib/private/Lock/NoopLockingProvider.php
index 94612e22f5..4f38d5159b 100644
--- a/lib/private/Lock/NoopLockingProvider.php
+++ b/lib/private/Lock/NoopLockingProvider.php
@@ -46,7 +46,7 @@ class NoopLockingProvider implements ILockingProvider {
 	/**
 	 * {@inheritdoc}
 	 */
-	public function acquireLock(string $path, int $type) {
+	public function acquireLock(string $path, int $type, string $readablePath = null) {
 		// do nothing
 	}
 
diff --git a/lib/public/Lock/ILockingProvider.php b/lib/public/Lock/ILockingProvider.php
index 34f69abdc0..6886b176c4 100644
--- a/lib/public/Lock/ILockingProvider.php
+++ b/lib/public/Lock/ILockingProvider.php
@@ -55,10 +55,11 @@ interface ILockingProvider {
 	/**
 	 * @param string $path
 	 * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE
+	 * @param string $readablePath human readable path to use in error messages, since 20.0.0
 	 * @throws \OCP\Lock\LockedException
 	 * @since 8.1.0
 	 */
-	public function acquireLock(string $path, int $type);
+	public function acquireLock(string $path, int $type, string $readablePath = null);
 
 	/**
 	 * @param string $path
diff --git a/lib/public/Lock/LockedException.php b/lib/public/Lock/LockedException.php
index 582157010c..4bd59c81b3 100644
--- a/lib/public/Lock/LockedException.php
+++ b/lib/public/Lock/LockedException.php
@@ -53,10 +53,15 @@ class LockedException extends \Exception {
 	 * @param string $path locked path
 	 * @param \Exception|null $previous previous exception for cascading
 	 * @param string $existingLock since 14.0.0
+	 * @param string $readablePath since 20.0.0
 	 * @since 8.1.0
 	 */
-	public function __construct(string $path, \Exception $previous = null, string $existingLock = null) {
-		$message = '"' . $path . '" is locked';
+	public function __construct(string $path, \Exception $previous = null, string $existingLock = null, string $readablePath = null) {
+		if ($readablePath) {
+			$message = "\"$path\"(\"$readablePath\") is locked";
+		} else {
+			$message = '"' . $path . '" is locked';
+		}
 		$this->existingLock = $existingLock;
 		if ($existingLock) {
 			$message .= ', existing lock on file: ' . $existingLock;