Skip to content

CDRIVER-4227 Add a shared_mutex, and use it when locking in shared_ptr atomic operations #895

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

Merged
merged 6 commits into from
Nov 17, 2021
Merged

CDRIVER-4227 Add a shared_mutex, and use it when locking in shared_ptr atomic operations #895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ff235ee
Add a shared_mutex, and use it when locking in shared_ptr atomic oper…
vector-of-bool Nov 11, 2021
72396da
Fix copy-paste error
vector-of-bool Nov 12, 2021
927f481
Tweak naming of shared mutex APIs
vector-of-bool Nov 15, 2021
ece6c8f
Force enable more recent POSIX APIs, regardless of language mode
vector-of-bool Nov 15, 2021
b6189eb
Tweak definition of _XOPEN_SOURCE
vector-of-bool Nov 15, 2021
a68cbac
No reallocf in strict POSIX mode
vector-of-bool Nov 16, 2021
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
57 changes: 56 additions & 1 deletion src/common/common-thread-private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ typedef struct {
#define bson_once_t INIT_ONCE
#define bson_thread_t HANDLE
#define BSON_THREAD_FUN(_function_name, _arg_name) \
unsigned(__stdcall _function_name) (void *(_arg_name))
unsigned (__stdcall _function_name) (void *(_arg_name))
#define BSON_THREAD_FUN_TYPE(_function_name) \
unsigned (__stdcall * _function_name) (void *)
#define BSON_THREAD_RETURN return 0
Expand All @@ -123,6 +123,61 @@ int COMMON_PREFIX (thread_create) (bson_thread_t *thread,
bool COMMON_PREFIX (mutex_is_locked) (bson_mutex_t *mutex);
#endif

/**
* @brief A shared mutex (a read-write lock)
*
* A shared mutex can be locked in 'shared' mode or 'exclusive' mode. Only one
* thread may hold exclusive mode at a time. Any number of threads may hold
* the lock in shared mode simultaneously. No thread can hold in exclusive mode
* while another thread holds in shared mode, and vice-versa.
*/
typedef struct bson_shared_mutex_t {
BSON_IF_WINDOWS (SRWLOCK native;)
BSON_IF_POSIX (pthread_rwlock_t native;)
Copy link
Contributor

Choose a reason for hiding this comment

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

to consider: Note that pthreads also has a pthread_mutex_t, which makes me wonder if bson_shared_mutex_t is the right name, especially as the platforms also make that distinction. bson_shared_rwlock? "Archibald"?

} bson_shared_mutex_t;

static BSON_INLINE void
bson_shared_mutex_init (bson_shared_mutex_t *mtx)
{
BSON_IF_WINDOWS (InitializeSRWLock (&mtx->native));
BSON_IF_POSIX (pthread_rwlock_init (&mtx->native, NULL));
}

static BSON_INLINE void
bson_shared_mutex_destroy (bson_shared_mutex_t *mtx)
{
BSON_IF_WINDOWS ((void) mtx;)
BSON_IF_POSIX (pthread_rwlock_destroy (&mtx->native);)
}

static BSON_INLINE void
bson_shared_mutex_lock_shared (bson_shared_mutex_t *mtx)
{
BSON_IF_WINDOWS (AcquireSRWLockShared (&mtx->native);)
BSON_IF_POSIX (pthread_rwlock_rdlock (&mtx->native);)
}

static BSON_INLINE void
bson_shared_mutex_lock (bson_shared_mutex_t *mtx)
{
BSON_IF_WINDOWS (AcquireSRWLockExclusive (&mtx->native);)
BSON_IF_POSIX (pthread_rwlock_wrlock (&mtx->native);)
}

static BSON_INLINE void
bson_shared_mutex_unlock (bson_shared_mutex_t *mtx)
{
BSON_IF_WINDOWS (ReleaseSRWLockExclusive (&mtx->native);)
BSON_IF_POSIX (pthread_rwlock_unlock (&mtx->native);)
}

static BSON_INLINE void
bson_shared_mutex_unlock_shared (bson_shared_mutex_t *mtx)
{
BSON_IF_WINDOWS (ReleaseSRWLockShared (&mtx->native);)
BSON_IF_POSIX (pthread_rwlock_unlock (&mtx->native);)
}

BSON_END_DECLS

#endif /* COMMON_THREAD_PRIVATE_H */
8 changes: 3 additions & 5 deletions src/libbson/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ set (BSON_API_VERSION 1.0)
set (CPACK_PACKAGE_VERSION_MAJOR ${BSON_MAJOR_VERSION})
set (CPACK_PACKAGE_VERSION_MINOR ${BSON_MINOR_VERSION})

# Enable X/Open 7.0 / POSIX 2008
add_compile_options($<$<NOT:$<PLATFORM_ID:Windows>>:-D_XOPEN_SOURCE=700>)

include (CPack)
TEST_BIG_ENDIAN (BSON_BIG_ENDIAN)

Expand All @@ -52,11 +55,6 @@ else ()
set (BSON_HAVE_SNPRINTF 1)
endif ()

CHECK_FUNCTION_EXISTS (reallocf BSON_HAVE_REALLOCF)
if (NOT BSON_HAVE_REALLOCF)
set (BSON_HAVE_REALLOCF 0)
endif ()

CHECK_STRUCT_HAS_MEMBER ("struct timespec" tv_sec time.h BSON_HAVE_TIMESPEC)
if (NOT BSON_HAVE_TIMESPEC)
message (STATUS " no timespec struct")
Expand Down
9 changes: 0 additions & 9 deletions src/libbson/src/bson/bson-config.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,6 @@
#endif


/*
* Define to 1 if you have reallocf available on your platform.
*/
#define BSON_HAVE_REALLOCF @BSON_HAVE_REALLOCF@
#if BSON_HAVE_REALLOCF != 1
# undef BSON_HAVE_REALLOCF
#endif


/*
* Define to 1 if you have struct timespec available on your platform.
*/
Expand Down
8 changes: 0 additions & 8 deletions src/libbson/src/bson/bson-memory.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,7 @@
static bson_mem_vtable_t gMemVtable = {
malloc,
calloc,
#ifdef BSON_HAVE_REALLOCF
reallocf,
#else
realloc,
#endif
free,
};

Expand Down Expand Up @@ -299,11 +295,7 @@ bson_mem_restore_vtable (void)
bson_mem_vtable_t vtable = {
malloc,
calloc,
#ifdef BSON_HAVE_REALLOCF
reallocf,
#else
realloc,
#endif
free,
};

Expand Down
24 changes: 6 additions & 18 deletions src/libmongoc/src/mongoc/mongoc-shared.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,27 +32,15 @@ _release_aux (_mongoc_shared_ptr_aux *aux)
bson_free (aux);
}

static bson_mutex_t g_shared_ptr_mtx;
static bson_shared_mutex_t g_shared_ptr_mtx;
static bson_once_t g_shared_ptr_mtx_init_once = BSON_ONCE_INIT;

static BSON_ONCE_FUN (_init_mtx)
{
bson_mutex_init (&g_shared_ptr_mtx);
bson_shared_mutex_init (&g_shared_ptr_mtx);
BSON_ONCE_RETURN;
}

static void
_shared_ptr_lock ()
{
bson_mutex_lock (&g_shared_ptr_mtx);
}

static void
_shared_ptr_unlock ()
{
bson_mutex_unlock (&g_shared_ptr_mtx);
}

void
mongoc_shared_ptr_reset (mongoc_shared_ptr *const ptr,
void *const pointee,
Expand Down Expand Up @@ -105,11 +93,11 @@ mongoc_atomic_shared_ptr_store (mongoc_shared_ptr *const out,
/* We are effectively "copying" the 'from' */
(void) mongoc_shared_ptr_copy (from);

_shared_ptr_lock ();
bson_shared_mutex_lock (&g_shared_ptr_mtx);
/* Do the exchange. Quick! */
prev = *out;
*out = from;
_shared_ptr_unlock ();
bson_shared_mutex_unlock (&g_shared_ptr_mtx);

/* Free the pointer that we just overwrote */
mongoc_shared_ptr_reset_null (&prev);
Expand All @@ -120,9 +108,9 @@ mongoc_atomic_shared_ptr_load (mongoc_shared_ptr const *ptr)
{
mongoc_shared_ptr r;
BSON_ASSERT_PARAM (ptr);
_shared_ptr_lock ();
bson_shared_mutex_lock_shared (&g_shared_ptr_mtx);
r = mongoc_shared_ptr_copy (*ptr);
_shared_ptr_unlock ();
bson_shared_mutex_unlock_shared (&g_shared_ptr_mtx);
return r;
}

Expand Down