Skip to content

Conversation

@icewind1991
Copy link
Member

This can be caused by the code releasing more locks then it acquires,
once the lock value becomes negative it's likely that it will never be able
to change into an exclusive lock again.

Signed-off-by: Robin Appelman robin@icewind.nl

This can be caused by the code releasing more locks then it acquires,
once the lock value becomes negative it's likely that it will never be able
to change into an exclusive lock again.

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Jul 5, 2018
@icewind1991 icewind1991 added this to the Nextcloud 14 milestone Jul 5, 2018
@enoch85
Copy link
Member

enoch85 commented Jul 5, 2018

Will this fix old locked files as well?

@icewind1991
Copy link
Member Author

If a shared lock is acquired and released for an existing broken file it should fix itself.

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code makes sense and it still works fine 👍

@MorrisJobke
Copy link
Member

The failing tests are unrelated -> merging.

@MorrisJobke MorrisJobke merged commit 422c805 into master Jul 6, 2018
@MorrisJobke MorrisJobke deleted the lock-negative branch July 6, 2018 12:51
@MorrisJobke
Copy link
Member

Backport to stable13 makes sense IMO.

@MorrisJobke
Copy link
Member

@icewind1991 We should do the same for the DB provider as I have seen a stak trace with "-1" in the SQL query as well: #9305 (comment)

@icewind1991
Copy link
Member Author

The db locking backend uses -1 to denote exclusive locks and the query for releasing shared locks already has a check to prevent it from going negative

@MorrisJobke
Copy link
Member

@icewind1991 Mind to open backport PRs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants