Make extensions thread-safe #5
Merged
Conversation
This file contains 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
This changes the implementation of the extension library to provide native types to the callback function. This avoids an extra call to MoveBlock, which reduces the chance of memory corruption.
This callback function utilizes a buffer and length parameter that can be provided as LStrHandle to remove the need for MoveBlock to be called in the event handler.
This matches the function signature from libssh2.
This replaces the global variable for the number of bytes send through lvssh2_session_callback_send_function() with a pointer value that is set by the event handler. By removing the global variable, the function becomes thread-safe as the state becomes thread-local.
This replaces the global variable for the number of bytes received with lvssh2_session_callback_recv() with a pointer to a value that is passed to the event handler, which makes the function thread-safe as all data is now thread-local.
This replaces the global variable with a pointer value used by the event callback handler. This makes the callback function thread-safe as all state data is thread-local.
This makes the code somewhat more readable and clarifies which value is pointer-type and which is value-type.
libssh2_session_init_ex() was broken because of a function wire mis- match on lvssh2_session_callback_receive_handler_32.vi caused by an incorrect type definition in the event data control. Fixed by using the correct type definition.
…er_32.vi This issue is caused by the MoveBlock operation which, on 32-bit platforms, should move 4 bytes instead of 8.
For the callback functions, it is generally not needed to allocate memory on the heap because the variables are only used inside the functions. There are some exceptions where data is returned to the caller (libssh2), in which case the caller is responsible to free the allocated memory.
Use LStrHandle when adding a response.
This changes the implementation for the keyboard-interactive response function to directly add responses to the array instead of using a helper function. This further ensures that all variables are thread- local, making the entire library thread-safe.
libssh2 defines several macros and typedefs for callback functions that we can utilize to ensure type-safety. This applies to all of the callback functions except lvssh2_trace_handler_function, for which libssh2 only defines a typedef. Nonetheless, using the typedef ensures type parity when returning the function pointer.
…unction in libssh2_session_init_ex() This removes the 'response' input from libssh2_userauth_keyboard_interactive_ex() and adds a new parameter 'keyboard-interactive response' to the abstract data provided to libssh2_session_init_ex() This is done so the callback can be registered to the abstract type that is provided to the callback handler, which makes keyboard- interactive authentication thread-safe as the operation is executed on the session object and not through a global variable. This makes the setup of keyboard-interactive authentication slightly more awkward as the callback handler must be registered during session initialization. However, it avoids any disambiguation when authenticating in multiple parallel threads.
When allocating memory for the signature, the type is `unsigned char`, not `unsigned char*`. This effectively reserves more memory than necessary (4/8 bytes instead of 1).
LStrHandle is limited to a size of INT32_MAX, while libssh2 can handle size_t. Since LabVIEW is unable to handle data of this size, we need to truncate the length to what LabVIEW can handle. Note that this can result in undefined behavior as the truncated data is incomplete. While it is very unlikely that anyone will ever reach the limits, the possibility is there. A possible workaround is to split the data into portions that LabVIEW can handle and post them individually. However, this makes the code much more complex with unlikely usecases.
Since libssh2 is written in C, it makes no sense to enforce C++ just because it is the default in Visual Studio. This changes the compiler language and several code constructs to C.
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 aims to make the extensions library thread-safe.