Threadsafe connection #1395
Draft
Threadsafe connection #1395
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The connection holder should be performant in all variants: 1. single-threaded use 2. opened once (open forever) 3. concurrent open/close Hence, a light-weight binary semaphore is used to synchronize opening and closing a database connection.
A user-provided 'on open' handler and connection control options can now be specified in a declarative way when making the 'storage' object.
trueqbit
force-pushed
the
experimental/threadsafe_connection
branch
from
564025d to
14a2aa0
Compare
fnc12
reviewed
Feb 11, 2025
trueqbit
force-pushed
the
experimental/threadsafe_connection
branch
from
f82faa4 to
3ed7507
Compare
... lost with merge commit f41c81a.
Simplified retaining connections for in-memory databases.
... which are the last remaining C++17 core language features
* Use a `std::mutex` in favor of `std::binary_semaphore` as it is most likely the better choice for the purpose of mutually exclusive locking. While it is much bigger in size and provides less `noexcept` guarantees, I didn't notice any difference in performance and it is readily available. Bigger performance gains can be achieved by other means. * This version of the connection holder properly handles recursive calls while the connection setup lock is held.
…ling * The variable storing the initializing thread id must be atomic. * Introduced a connection pointer in order to be able to handle both, connected and unconnected states.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With this PR a
storageinstance can be safely shared, i.e. opening, closing or retaining database connections are thread-safe.Previously this was only the case if the database was opened permanently ("forever").
Note that other shared configuration settings or objects, such as limits or application-defined functions, can still only be managed in a single-thread context, which makes sense anyway.
Establishing a connection should be performant in all variants:
I tested the new core logic with a test example and also checked my thoughts this time with a Claude conversation.
Note: One important thing necessary to discuss is the phase of setting up a database connection within a user-provided
on_opencallback that captures thestorageinstance and in turn access the freshly opened database - like creating application-defined functions, setting pragma and limit values or querying database properties:Currently option 1. is implemented, which involves checking the thread id of the recursively executing thread.
Additionally I'd like to extend the wiki or documentation somewhere about
thread-safety.md.
This PR would resolve the following issues:
#207
#1053 [With test example that I verified!]
This PR would partially resolve the following issues, due to internal connection handling now being thread-safe:
#163
#707
#804
#875
#1274