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.

Sign up for GitHub

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

Fix Race Condition #637

Merged
merged 7 commits into from
May 5, 2023
Merged

Fix Race Condition #637

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0981859
fix race condition by moving critical section into the mutex protecte…
kkloberdanz May 3, 2023
7fb1687
fmt
kkloberdanz May 3, 2023
d6a8d9b
simplify by avoiding copying of state
kkloberdanz May 4, 2023
f50e64e
dlclose seems to be working without issue on Linux
kkloberdanz May 4, 2023
3d06700
Revert "dlclose seems to be working without issue on Linux"
kkloberdanz May 4, 2023
73d0243
link to ticket explaining issue with dlclose
kkloberdanz May 5, 2023
9b32ca2
prevent double-free
kkloberdanz May 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions src/mongocrypt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,27 +606,27 @@ static void _csfle_drop_global_ref(void) {
old_state = g_csfle_state;
dropped_last_ref = true;
}
}

if (dropped_last_ref) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The reason for this boolean and copying the old state was to avoid doing this work within the protection of the mutex. If this is moved into that scope, then the code can be simplified to do the destruction directly without the state copying.

mongo_crypt_v1_status *status = old_state.vtable.status_create();
const int destroy_rc = old_state.vtable.lib_destroy(old_state.csfle_lib, status);
if (destroy_rc != MONGO_CRYPT_V1_SUCCESS && status) {
fprintf(stderr,
"csfle lib_destroy() failed: %s [Error %d, code %d]\n",
old_state.vtable.status_get_explanation(status),
old_state.vtable.status_get_error(status),
old_state.vtable.status_get_code(status));
}
old_state.vtable.status_destroy(status);
if (dropped_last_ref) {
mongo_crypt_v1_status *status = old_state.vtable.status_create();
const int destroy_rc = old_state.vtable.lib_destroy(old_state.csfle_lib, status);
if (destroy_rc != MONGO_CRYPT_V1_SUCCESS && status) {
fprintf(stderr,
"csfle lib_destroy() failed: %s [Error %d, code %d]\n",
old_state.vtable.status_get_explanation(status),
old_state.vtable.status_get_error(status),
old_state.vtable.status_get_code(status));
}
old_state.vtable.status_destroy(status);

#ifndef __linux__
mcr_dll_close(old_state.dll);
mcr_dll_close(old_state.dll);
#endif
/// NOTE: On Linux, skip closing the CSFLE library itself, since a bug in
/// the way ld-linux and GCC interact causes static destructors to not run
/// during dlclose(). Still, free the error string:
mstr_free(old_state.dll.error_string);
/// NOTE: On Linux, skip closing the CSFLE library itself, since a bug in
/// the way ld-linux and GCC interact causes static destructors to not run
/// during dlclose(). Still, free the error string:
mstr_free(old_state.dll.error_string);
}
}
}

Expand Down