Fix sharedstorage recursion hell #25799

PVince81 · 2016-08-15T09:22:20Z

Forward port of #25789

To prevent recursions in initMountPoints which requires the numeric id to populate oc_mounts

PVince81 · 2016-08-15T09:23:08Z

@osadmin since you're likely on git, you can check out the branch "fix-sharedstorage-recursion-hell" to test this. If not, you can download it as a patch for 9.2: https://github.com/owncloud/core/pull/25799.patch

osadmin · 2016-08-16T09:01:55Z

Hi. I've just tried to patch my owncloud-files-9.2.0-0.1.1.prealpha.20160812.noarch

patch -p1 --dry-run < ./25799.patch --verbose
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|From b719a55d8944273d8278118b0fe4739ed26786e7 Mon Sep 17 00:00:00 2001
|From: Vincent Petry <pvince81@owncloud.com>
|Date: Wed, 10 Aug 2016 08:26:00 +0200
|Subject: [PATCH 1/4] Lazy init shared storage by tweaking jail
|
|---
| apps/files_sharing/lib/SharedMount.php        |   9 ++
| apps/files_sharing/lib/sharedstorage.php      |  18 ++--
| lib/private/Files/Storage/Wrapper/Jail.php    |  88 +++++++++++---------
| lib/private/Files/Storage/Wrapper/Wrapper.php | 113 +++++++++++++-------------
| 4 files changed, 127 insertions(+), 101 deletions(-)
|
|diff --git a/apps/files_sharing/lib/SharedMount.php b/apps/files_sharing/lib/SharedMount.php
|index 896da35..fd99722 100644
|--- a/apps/files_sharing/lib/SharedMount.php
|+++ b/apps/files_sharing/lib/SharedMount.php
--------------------------
Patching file apps/files_sharing/lib/SharedMount.php using Plan A...
Hunk #1 succeeded at 234.
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php
|index 85e2086..729cf23 100644
|--- a/apps/files_sharing/lib/sharedstorage.php
|+++ b/apps/files_sharing/lib/sharedstorage.php
--------------------------
Patching file apps/files_sharing/lib/sharedstorage.php using Plan A...
Hunk #1 succeeded at 82.
Hunk #2 succeeded at 98.
Hunk #3 succeeded at 434 (offset -5 lines).
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/lib/private/Files/Storage/Wrapper/Jail.php b/lib/private/Files/Storage/Wrapper/Jail.php
|index ecaac80..7f368d0 100644
|--- a/lib/private/Files/Storage/Wrapper/Jail.php
|+++ b/lib/private/Files/Storage/Wrapper/Jail.php
--------------------------
Patching file lib/private/Files/Storage/Wrapper/Jail.php using Plan A...
Hunk #1 succeeded at 45.
Hunk #2 succeeded at 72.
Hunk #3 succeeded at 82.
Hunk #4 succeeded at 92.
Hunk #5 succeeded at 102.
Hunk #6 succeeded at 112.
Hunk #7 succeeded at 123.
Hunk #8 succeeded at 133.
Hunk #9 succeeded at 144.
Hunk #10 succeeded at 154.
Hunk #11 succeeded at 164.
Hunk #12 succeeded at 174.
Hunk #13 succeeded at 184.
Hunk #14 succeeded at 194.
Hunk #15 succeeded at 205.
Hunk #16 succeeded at 215.
Hunk #17 succeeded at 225.
Hunk #18 succeeded at 235.
Hunk #19 succeeded at 246.
Hunk #20 succeeded at 256.
Hunk #21 succeeded at 267.
Hunk #22 succeeded at 278.
Hunk #23 succeeded at 289.
Hunk #24 succeeded at 300.
Hunk #25 succeeded at 312.
Hunk #26 succeeded at 322.
Hunk #27 succeeded at 332.
Hunk #28 succeeded at 344.
Hunk #29 succeeded at 355.
Hunk #30 succeeded at 369.
Hunk #31 succeeded at 380.
Hunk #32 succeeded at 397.
Hunk #33 succeeded at 411.
Hunk #34 succeeded at 421.
Hunk #35 succeeded at 429.
Hunk #36 succeeded at 439.
Hunk #37 succeeded at 448.
Hunk #38 succeeded at 457.
Hunk #39 succeeded at 467.
Hunk #40 succeeded at 480.
Hunk #41 succeeded at 493.
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/lib/private/Files/Storage/Wrapper/Wrapper.php b/lib/private/Files/Storage/Wrapper/Wrapper.php
|index 00f6211..a5541b2 100644
|--- a/lib/private/Files/Storage/Wrapper/Wrapper.php
|+++ b/lib/private/Files/Storage/Wrapper/Wrapper.php
--------------------------
Patching file lib/private/Files/Storage/Wrapper/Wrapper.php using Plan A...
Hunk #1 succeeded at 63.
Hunk #2 succeeded at 73.
Hunk #3 succeeded at 83.
Hunk #4 succeeded at 93.
Hunk #5 succeeded at 103.
Hunk #6 succeeded at 113.
Hunk #7 succeeded at 124.
Hunk #8 succeeded at 134.
Hunk #9 succeeded at 145.
Hunk #10 succeeded at 155.
Hunk #11 succeeded at 165.
Hunk #12 succeeded at 175.
Hunk #13 succeeded at 185.
Hunk #14 succeeded at 195.
Hunk #15 succeeded at 206.
Hunk #16 succeeded at 216.
Hunk #17 succeeded at 226.
Hunk #18 succeeded at 236.
Hunk #19 succeeded at 247.
Hunk #20 succeeded at 257.
Hunk #21 succeeded at 268.
Hunk #22 succeeded at 279.
Hunk #23 succeeded at 290.
Hunk #24 succeeded at 301.
Hunk #25 succeeded at 313.
Hunk #26 succeeded at 323.
Hunk #27 succeeded at 333.
Hunk #28 succeeded at 345.
Hunk #29 succeeded at 356.
Hunk #30 succeeded at 370.
Hunk #31 succeeded at 384.
Hunk #32 succeeded at 398.
Hunk #33 succeeded at 409.
Hunk #34 succeeded at 423.
Hunk #35 succeeded at 454.
Hunk #36 succeeded at 463.
Hunk #37 succeeded at 472.
Hunk #38 succeeded at 482.
Hunk #39 succeeded at 493.
Hunk #40 succeeded at 505.
Hunk #41 succeeded at 514.
Hunk #42 succeeded at 523.
Hunk #43 succeeded at 533.
Hunk #44 succeeded at 547.
Hunk #45 succeeded at 561.
Hunk #46 succeeded at 569.
Hunk #47 succeeded at 579.
Hunk #48 succeeded at 591.
Hunk #49 succeeded at 603.
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|
|From b72618bfb49934fbc8ab7944d47d4d48fda3d29f Mon Sep 17 00:00:00 2001
|From: Vincent Petry <pvince81@owncloud.com>
|Date: Wed, 10 Aug 2016 10:32:25 +0200
|Subject: [PATCH 2/4] Get shared storage numeric id directly from DB
|
|To prevent recursions in initMountPoints which requires the numeric id
|to populate oc_mounts
|---
| lib/private/Files/Config/LazyStorageMountInfo.php | 12 ++++++++----
| 1 file changed, 8 insertions(+), 4 deletions(-)
|
|diff --git a/lib/private/Files/Config/LazyStorageMountInfo.php b/lib/private/Files/Config/LazyStorageMountInfo.php
|index a3cac99..ad00dbe 100644
|--- a/lib/private/Files/Config/LazyStorageMountInfo.php
|+++ b/lib/private/Files/Config/LazyStorageMountInfo.php
--------------------------
Patching file lib/private/Files/Config/LazyStorageMountInfo.php using Plan A...
Hunk #1 succeeded at 47.
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|
|From 7f8128548179c9b08e502c0979e1710d4d08af8f Mon Sep 17 00:00:00 2001
|From: Vincent Petry <pvince81@owncloud.com>
|Date: Thu, 11 Aug 2016 10:45:05 +0200
|Subject: [PATCH 3/4] Flag to not recurse into shared mounts in getPath
|
|---
| apps/files_sharing/lib/sharedstorage.php | 2 +-
| lib/private/Files/View.php               | 9 ++++++++-
| 2 files changed, 9 insertions(+), 2 deletions(-)
|
|diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php
|index 729cf23..30d5277 100644
|--- a/apps/files_sharing/lib/sharedstorage.php
|+++ b/apps/files_sharing/lib/sharedstorage.php
--------------------------
Patching file apps/files_sharing/lib/sharedstorage.php using Plan A...
Hunk #1 succeeded at 99 with fuzz 1 (offset 4 lines).
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php
|index 337d188..4921fb4 100644
|--- a/lib/private/Files/View.php
|+++ b/lib/private/Files/View.php
--------------------------
Patching file lib/private/Files/View.php using Plan A...
Hunk #1 succeeded at 62.
Hunk #2 succeeded at 1671.
Hunk #3 succeeded at 1687.
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|
|From 40e66ccba22156b23938fd58852895ba5eb48bd5 Mon Sep 17 00:00:00 2001
|From: Vincent Petry <pvince81@owncloud.com>
|Date: Thu, 11 Aug 2016 11:13:50 +0200
|Subject: [PATCH 4/4] Use FailedStorage when share is invalid
|
|---
| apps/files_sharing/lib/sharedstorage.php | 7 ++++---
| 1 file changed, 4 insertions(+), 3 deletions(-)
|
|diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php
|index 30d5277..4047692 100644
|--- a/apps/files_sharing/lib/sharedstorage.php
|+++ b/apps/files_sharing/lib/sharedstorage.php
--------------------------
Patching file apps/files_sharing/lib/sharedstorage.php using Plan A...
Hunk #1 succeeded at 37.
Hunk #2 FAILED at 100.
Hunk #3 succeeded at 312.
Hunk #4 succeeded at 434 with fuzz 2 (offset -11 lines).
1 out of 4 hunks FAILED -- saving rejects to file apps/files_sharing/lib/sharedstorage.php.rej
done

PVince81 · 2016-08-16T09:11:02Z

You might need to update to the latest prealpha first, your version seems to be from a few days ago where other changes were made to sharedstorage.

PVince81 · 2016-08-16T11:43:01Z

@SergioBertolinSG can you run your big test with 1000 users like in the other PR ? This is the master PR. Thanks.

@jvillafanez quick rereview for this forward port of #25789 (comment) ?

SergioBertolinSG · 2016-08-16T11:44:05Z

Sure.

jvillafanez · 2016-08-16T11:53:32Z

Code looks good 👍

SergioBertolinSG · 2016-08-16T14:20:27Z

Same behaviour as stable9.1. No errors, seems to work. 👍

But there is an important performance problem, users cannot login and users won't appear quickly. (This works eventually, but it requires a lot of time). Please note that I am not using any cache system.

PVince81 · 2016-08-16T14:23:18Z

@SergioBertolinSG is the perf issue worse than on the stable9.1 version ?

SergioBertolinSG · 2016-08-16T14:29:58Z

No, the same, it just depends on how much users share folders with the big group. More shares with group => more slow login and user management page loading.

PVince81 · 2016-08-16T14:32:08Z

Ok, I think the perf issue is anyway unrelated and might happen on OC 9.0 too.

PVince81 · 2016-08-16T14:45:18Z

Pfff, why does this fail randomly... passes locally. Merging.

PVince81 · 2016-08-16T15:18:09Z

Smashbox tests passed locally (just in case)

sonanchenko · 2016-08-21T05:30:48Z

Hi,
I've just updated to 9.2.0-0.1.1.prealpha.20160819
tried to apply the patch, but seems changes are in place already.

Unfortunately I still suffering from the same for my https - no way to even log in: "CSRF check failed"

@PVince81 could you please suggest on what debug info can I gather for you ?

PVince81 · 2016-08-22T07:03:14Z

@sonanchenko if 9.1.1RC1 or 9.2 prealpha do not work for you then the problem you have is unrelated. This ticket here is about memory issues causing a "CSRF" message only as a side-effect.

@sonanchenko can you make a new ticket using the issue template https://raw.githubusercontent.com/owncloud/core/master/issue_template.md ?

lock · 2019-08-04T23:01:45Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Vincent Petry added 4 commits August 15, 2016 11:18

Lazy init shared storage by tweaking jail

b719a55

Get shared storage numeric id directly from DB

b72618b

To prevent recursions in initMountPoints which requires the numeric id to populate oc_mounts

Flag to not recurse into shared mounts in getPath

7f81285

Use FailedStorage when share is invalid

40e66cc

PVince81 added 3 - To Review feature:sharing labels Aug 15, 2016

PVince81 added this to the 9.2 milestone Aug 15, 2016

PVince81 mentioned this pull request Aug 15, 2016

File Sharing stack overflow, memory issue, crash, CSRF issue on 9.1.0 #25557

Closed

PVince81 merged commit 9975240 into master Aug 16, 2016

PVince81 deleted the fix-sharedstorage-recursion-hell branch August 16, 2016 14:45

rullzer mentioned this pull request Aug 19, 2016

Cannot log in after upgrade to NC10 RC1, php-fpm segfaults nextcloud/server#986

Closed

icewind1991 mentioned this pull request Aug 22, 2016

Fix shared storage recursive setup nextcloud/server#1007

Merged

sonanchenko mentioned this pull request Aug 24, 2016

CSRF check failed #25927

Closed

lock bot locked as resolved and limited conversation to collaborators Aug 4, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix sharedstorage recursion hell #25799

Fix sharedstorage recursion hell #25799

PVince81 commented Aug 15, 2016

PVince81 commented Aug 15, 2016

osadmin commented Aug 16, 2016 •

edited by PVince81

Loading

PVince81 commented Aug 16, 2016

PVince81 commented Aug 16, 2016

SergioBertolinSG commented Aug 16, 2016

jvillafanez commented Aug 16, 2016

SergioBertolinSG commented Aug 16, 2016 •

edited

Loading

PVince81 commented Aug 16, 2016

SergioBertolinSG commented Aug 16, 2016 •

edited

Loading

PVince81 commented Aug 16, 2016

PVince81 commented Aug 16, 2016

PVince81 commented Aug 16, 2016

sonanchenko commented Aug 21, 2016

PVince81 commented Aug 22, 2016

lock bot commented Aug 4, 2019

Fix sharedstorage recursion hell #25799

Fix sharedstorage recursion hell #25799

Conversation

PVince81 commented Aug 15, 2016

PVince81 commented Aug 15, 2016

osadmin commented Aug 16, 2016 • edited by PVince81 Loading

PVince81 commented Aug 16, 2016

PVince81 commented Aug 16, 2016

SergioBertolinSG commented Aug 16, 2016

jvillafanez commented Aug 16, 2016

SergioBertolinSG commented Aug 16, 2016 • edited Loading

PVince81 commented Aug 16, 2016

SergioBertolinSG commented Aug 16, 2016 • edited Loading

PVince81 commented Aug 16, 2016

PVince81 commented Aug 16, 2016

PVince81 commented Aug 16, 2016

sonanchenko commented Aug 21, 2016

PVince81 commented Aug 22, 2016

lock bot commented Aug 4, 2019

osadmin commented Aug 16, 2016 •

edited by PVince81

Loading

SergioBertolinSG commented Aug 16, 2016 •

edited

Loading

SergioBertolinSG commented Aug 16, 2016 •

edited

Loading