Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check if the read lock is held before exiting #1639

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

huyhu
Copy link

@huyhu huyhu commented Apr 17, 2020

Check if the read lock is held before exiting.

Check if the read lock is held before exiting.
@huyhu huyhu changed the title #1637 Check if the read lock is held before exiting Apr 17, 2020
@@ -10,7 +10,7 @@ namespace LiteDB.Tests.Engine
{
public class ParallelQuery_Tests
{
[Fact(Skip = "Must fix parallel query fetch")]
Copy link
Author

Choose a reason for hiding this comment

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

Voila! the parallel query works with this change.

@@ -54,7 +54,8 @@ public void ExitTransaction()
// if current thread are in reserved mode, do not exit transaction (will be exit from ExitExclusive)
if (_transaction.IsWriteLockHeld) return;

_transaction.ExitReadLock();
if (_transaction.IsReadLockHeld)
Copy link
Author

Choose a reason for hiding this comment

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

Check before exiting, otherwise ExitReadLock throws System.Threading.SynchronizationLockException:The read lock is being released without being held

@lbnascimento
Copy link
Collaborator

@huyhu Does this actually fix your use case? Do you have any tests for this PR? I'm worried this PR only masks an underlying issue (why did the transaction try to release a lock that it didn't hold in the first place?)

@dethknite
Copy link

dethknite commented Jun 28, 2022

I would agree it is important to find the underlying cause of the issue, however, adding this check does stop a fatal exception that takes down an application. I, too, have encountered this error several times in the past few months, and I have not been able to isolate it to anything in particular. (I am using the mutex locking scheme--ie. direct--with async tasks, request the least permission necessary for db activity, and wait retries where applicable).

Perhaps adding -- if (held) {remove} / else {log} -- type of solution would suffice without reviewing all the locking logic and retaining this bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants