Multithreading support #16
Merged
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 async API is not supported -- implement all attributes that signal the lack of support. The driver supports the multi-threading model with connection/statement per thread and will further implement connection pooling
All handles have now their own lock, which is acquired at API entry. (The descriptor handles have it too, but the statement locks are used instead.) The connection handles have an extra lock for serializing access to the libcurl-related members, since multiple statements can be active for one connection.
Closed
droberts195
approved these changes
Aug 28, 2018
There was a problem hiding this comment.
LGTM
I only saw one typo.
For this sort of change any bugs are likely to be in what hasn't been changed rather than what has, so I guess the most important thing before release is to do some stress testing with many threads using the driver simultaneously to smoke out any places where locking should have been added but hasn't. But that can be done later, so I'm happy for this to be merged now.
driver/odbc.c
Outdated
| @@ -71,7 +71,7 @@ BOOL WINAPI DllMain( | |||
|
|
|||
| // Perform any necessary cleanup. | |||
| case DLL_PROCESS_DETACH: | |||
| INFO("process %u dettached.", GetCurrentProcessId()); | |||
| INFO("process %u dettaching.", GetCurrentProcessId()); | |||
There was a problem hiding this comment.
There should only be one 't' in 'detaching'.
s/dettaching/detaching/
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.
This PR adds the necessary mutexes for multi-threaded operation.
There are two types of mutexes being added:
Along this PR the async API is dropped (it never has been actually implemented, but there were a few handle members added for future implementation). This is likely not going to be implemented, most apps going for connection threading and pooling. (Relevant other drivers don't support it either.)