Skip to content

CBL-7717 : Lockmanager resources release on shutdown#451

Merged
Aniket392 merged 4 commits intomasterfrom
pr/CBL-7717
Jan 6, 2026
Merged

CBL-7717 : Lockmanager resources release on shutdown#451
Aniket392 merged 4 commits intomasterfrom
pr/CBL-7717

Conversation

@Aniket392
Copy link
Copy Markdown
Contributor

  • LockManager uses a RefQueueCleanerThread to monitor for garbage-collected locks.
  • This thread is created as non-daemon to ensure proper cleanup , but it prevents JVM exit until all locks are released.
  • When databases or other peer objects aren't explicitly closed, their locks remain in the LockManager's locks map , causing the thread to wait indefinitely.
  • Clears all remaining locks from the map
  • Stops the cleaner thread by calling its quit() method
  • Logs the shutdown operation for debugging

@Aniket392 Aniket392 requested a review from pasin January 5, 2026 11:39
@pasin pasin requested a review from borrrden January 5, 2026 17:47
if (t != null) {
t.quit();
}
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A couple of things to discuss :

  1. This doesn't prevent a new lock to be added (e.g. there is another thread acquiring the lock at the same time). Should we throw an exception if there is a request for a new lock after the shutdown is called?

  2. Is using reference queue to clean up lock objects really needed? I understand that we start to use the cleaner and reference queue to allow the native objects to be cleaned on non GC thread, but these are just simple objects.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  1. I have added to prevent this race condition
  2. Yes, the ReferenceQueue is essential for LockManager they represent locks on native peer resources (C4Database, C4Collection, etc.) that must be freed regardless of Java object lifecycle. Without ReferenceQueue cleanup, if a database object is garbage collected without explicit closure, the native peer remains locked and the LockManager thread waits indefinitely, preventing JVM exit

Copy link
Copy Markdown
Collaborator

@pasin pasin Jan 5, 2026

Choose a reason for hiding this comment

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

After spending time tryin to understand this, it's perfectly fine to use ReferenceQueue to clean up the LockRef in the locks map and to shutdown the queue thread.

Alternatively, instead of adding a specific shutdown method, the created RefQueueCleanerThread can be a daemon thread as :

  1. There is no specific resources to clean up except removing a weak object from the map.
  2. Based on my understanding, it seems like in C4Peer, we are using CBLExecutor which creates daemon threads for cleaning native peers.

}
}

public static void shutdown() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this be marked with @synchronized or something? Or does that not work with static methods?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@synchronized annotation is generally used in kotlin
For java we can use synchronized keyword as public static synchronized void shutdown()(but we can use some library to use annotation)

It works same as synchronized block, which we have used in shutdown, only difference being that synchronized keyword synchronizes the entire method, so only one thread at a time can execute any static synchronized method of that class
On other hand if we use synchronized block, we can specifically mention on which object we want to put lock on

Copy link
Copy Markdown
Member

@borrrden borrrden left a comment

Choose a reason for hiding this comment

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

Left one small comment

@Aniket392 Aniket392 requested a review from pasin January 6, 2026 11:07
@Aniket392 Aniket392 merged commit c90d4db into master Jan 6, 2026
@Aniket392 Aniket392 deleted the pr/CBL-7717 branch January 6, 2026 17:26
Aniket392 added a commit that referenced this pull request Jan 6, 2026
* CBL-7717 : Lockmanager resources release on shutdown

* Resolving the race condition issue

* Going with setting RefQueueCleanerThread as daemon

* Removed unnecessary spaces and imports

(cherry picked from commit c90d4db)
Aniket392 added a commit that referenced this pull request Jan 6, 2026
* CBL-7717 : Lockmanager resources release on shutdown

* Resolving the race condition issue

* Going with setting RefQueueCleanerThread as daemon

* Removed unnecessary spaces and imports

(cherry picked from commit c90d4db)
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