[stable33] fix: improve share mount conflict resolution logic #57822
Conversation
icewind1991
requested review from
artonge,
leftybournes,
nfebe and
salmart-dev
and removed request for
a team
github-actions
bot
changed the title
icewind1991
force-pushed
the
share-mount-validation-fixes-33
branch
5 times, most recently
from
577c711 to
b92f1c1
Compare
icewind1991
force-pushed
the
share-mount-validation-fixes-33
branch
from
b92f1c1 to
7de1833
Compare
| $validShareCache = $this->cacheFactory->createLocal('share-valid-mountpoint-max'); | ||
| $maxValidatedShare = $validShareCache->get($userId) ?? 0; | ||
| $newMaxValidatedShare = $maxValidatedShare; |
There was a problem hiding this comment.
I think this valid share cache based on an id offset is dangerous and can lead to race conditions.
If 2 request concurrently add shares, one may have ids 1 and 3, and validate both, and the other one 1 and 2, and will skip validation of 2 because the last validated share id is 3.
I would feel a lot better if this cache was a map of validated shares. Maybe that’s not possible for performance reasons but then we need another solution, this is too fragile.
There was a problem hiding this comment.
I don't think there is a case where getting the shares will return 3 but not 2? with auto-increment that isn't inside a transaction.
Storing the map of validated shares put to much of a load on the cache.
The logic has been working (as far as I know) fine since 32, this just moves it around a bit.
icewind1991
force-pushed
the
share-mount-validation-fixes-33
branch
from
7de1833 to
7fa5ada
Compare
|
Also added a backport of 43a9335 to stop some false positives |
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
…e positives Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
icewind1991
force-pushed
the
share-mount-validation-fixes-33
branch
from
7fa5ada to
d0d38e4
Compare
This backports the improved mount conflict resolution logic from #57295 but keeps it at mount-time.
The old logic way of checking for conflict was causing a recursive filesystem setup which lead to false positives in the conflict detection.
Fixes #57748