From ab44fc87ca89ad3477d51f2c85d99422a73010a9 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Wed, 13 Sep 2023 20:35:08 +0000 Subject: [PATCH] Split up compilation of rwlock to separate files. It is much easier to follow this way. Signed-off-by: Chris Lalancette --- CMakeLists.txt | 16 +++- src/logging.c | 6 ++ src/rwlock_pthread.c | 126 +++++++++++++++++++++++++++++ src/rwlock_stub.c | 74 +++++++++++++++++ src/{rwlock.c => rwlock_win32.c} | 133 ++++--------------------------- 5 files changed, 235 insertions(+), 120 deletions(-) create mode 100644 src/rwlock_pthread.c create mode 100644 src/rwlock_stub.c rename src/{rwlock.c => rwlock_win32.c} (58%) diff --git a/CMakeLists.txt b/CMakeLists.txt index 38ae04f9..4d81b562 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -42,10 +42,24 @@ if(NOT WIN32) add_compile_options(-Wall -Wextra -Wconversion -Wno-sign-conversion -Wpedantic) endif() +option(FORCE_STUB_RWLOCK "Force the use of a 'stub' rwlock implementation, which makes logging slower and inherently racy." OFF) + if(WIN32) set(time_impl_c src/time_win32.c) + set(rwlock_impl_c src/rwlock_win32.c) else() set(time_impl_c src/time_unix.c) + find_package(Threads QUIET) + if(Threads_FOUND) + set(rwlock_impl_c src/rwlock_pthread.c) + else() + if(FORCE_STUB_RWLOCK) + set(rwlock_impl_c src/rwlock_stub.c) + add_definitions(-DRWLOCK_STUB) + else() + message(FATAL "No valid rwlock implementation found!") + endif() + endif() endif() set(rcutils_sources @@ -63,7 +77,7 @@ set(rcutils_sources src/process.c src/qsort.c src/repl_str.c - src/rwlock.c + ${rwlock_impl_c} src/sha256.c src/shared_library.c src/snprintf.c diff --git a/src/logging.c b/src/logging.c index 90b4359f..7a183bcd 100644 --- a/src/logging.c +++ b/src/logging.c @@ -983,6 +983,11 @@ int rcutils_logging_get_logger_effective_level(const char * name) severity = g_rcutils_logging_default_logger_level; } +#if !defined(RWLOCK_STUB) + // If the RWLOCK implementation is a stub, this will definitely cause corruption, even between get + // calls, so we skip it (the consequence is that log level lookups are slower). With a stub + // RWLOCK there is still a race between get and set, but that is less common. + // If the calculated severity is anything but UNSET, we place it into the hashmap for speedier // lookup next time. If the severity is UNSET, we don't bother because we are going to have to // walk the hierarchy next time anyway. @@ -995,6 +1000,7 @@ int rcutils_logging_get_logger_effective_level(const char * name) "Failed to cache severity; this is not fatal but will impact performance\n"); } } +#endif return severity; } diff --git a/src/rwlock_pthread.c b/src/rwlock_pthread.c new file mode 100644 index 00000000..dd35ff99 --- /dev/null +++ b/src/rwlock_pthread.c @@ -0,0 +1,126 @@ +// Copyright 2023 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include + +#include "./rwlock.h" // NOLINT + +#include "rcutils/allocator.h" +#include "rcutils/error_handling.h" +#include "rcutils/types/rcutils_ret.h" + +typedef struct rcutils_rwlock_impl_s +{ + pthread_rwlock_t lock; + rcutils_allocator_t allocator; +} rcutils_rwlock_impl_t; + +rcutils_rwlock_t +rcutils_get_zero_initialized_rwlock(void) +{ + static rcutils_rwlock_t zero_initialized_rwlock; + zero_initialized_rwlock.impl = NULL; + return zero_initialized_rwlock; +} + +rcutils_ret_t +rcutils_rwlock_init(rcutils_rwlock_t * lock, rcutils_allocator_t allocator) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(lock, RCUTILS_RET_INVALID_ARGUMENT); + if (lock->impl != NULL) { + RCUTILS_SET_ERROR_MSG("rwlock already initialized"); + return RCUTILS_RET_ERROR; + } + RCUTILS_CHECK_ALLOCATOR_WITH_MSG( + &allocator, "invalid allocator", return RCUTILS_RET_INVALID_ARGUMENT); + + lock->impl = allocator.allocate(sizeof(rcutils_rwlock_impl_t), allocator.state); + if (NULL == lock->impl) { + RCUTILS_SET_ERROR_MSG("failed to allocate memory for string map impl struct"); + return RCUTILS_RET_BAD_ALLOC; + } + + lock->impl->allocator = allocator; + + return pthread_rwlock_init(&lock->impl->lock, NULL) == 0 ? RCUTILS_RET_OK : RCUTILS_RET_ERROR; +} + +rcutils_ret_t +rcutils_rwlock_read_lock(rcutils_rwlock_t * lock) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(lock, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_FOR_NULL_WITH_MSG( + lock->impl, "invalid lock", return RCUTILS_RET_ERROR); + + return pthread_rwlock_rdlock(&lock->impl->lock) == 0 ? RCUTILS_RET_OK : RCUTILS_RET_ERROR; +} + +rcutils_ret_t +rcutils_rwlock_read_unlock(rcutils_rwlock_t * lock) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(lock, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_FOR_NULL_WITH_MSG( + lock->impl, "invalid lock", return RCUTILS_RET_ERROR); + + return pthread_rwlock_unlock(&lock->impl->lock) == 0 ? RCUTILS_RET_OK : RCUTILS_RET_ERROR; +} + +rcutils_ret_t +rcutils_rwlock_write_trylock(rcutils_rwlock_t * lock) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(lock, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_FOR_NULL_WITH_MSG( + lock->impl, "invalid lock", return RCUTILS_RET_ERROR); + + return pthread_rwlock_trywrlock(&lock->impl->lock) == 0 ? RCUTILS_RET_OK : RCUTILS_RET_ERROR; +} + +rcutils_ret_t +rcutils_rwlock_write_lock(rcutils_rwlock_t * lock) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(lock, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_FOR_NULL_WITH_MSG( + lock->impl, "invalid lock", return RCUTILS_RET_ERROR); + + return pthread_rwlock_wrlock(&lock->impl->lock) == 0 ? RCUTILS_RET_OK : RCUTILS_RET_ERROR; +} + +rcutils_ret_t +rcutils_rwlock_write_unlock(rcutils_rwlock_t * lock) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(lock, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_FOR_NULL_WITH_MSG( + lock->impl, "invalid lock", return RCUTILS_RET_ERROR); + + return pthread_rwlock_unlock(&lock->impl->lock) == 0 ? RCUTILS_RET_OK : RCUTILS_RET_ERROR; +} + +rcutils_ret_t +rcutils_rwlock_fini(rcutils_rwlock_t * lock) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(lock, RCUTILS_RET_INVALID_ARGUMENT); + if (NULL == lock->impl) { + return RCUTILS_RET_OK; + } + + int retval = pthread_rwlock_destroy(&lock->impl->lock); + + rcutils_allocator_t allocator = lock->impl->allocator; + + allocator.deallocate(lock->impl, allocator.state); + lock->impl = NULL; + + return retval == 0 ? RCUTILS_RET_OK : RCUTILS_RET_ERROR; +} diff --git a/src/rwlock_stub.c b/src/rwlock_stub.c new file mode 100644 index 00000000..9a5ef44c --- /dev/null +++ b/src/rwlock_stub.c @@ -0,0 +1,74 @@ +// Copyright 2023 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "./rwlock.h" // NOLINT + +#include "rcutils/allocator.h" +#include "rcutils/types/rcutils_ret.h" + +rcutils_rwlock_t +rcutils_get_zero_initialized_rwlock(void) +{ + return RCUTILS_RET_OK; +} + +rcutils_ret_t +rcutils_rwlock_init(rcutils_rwlock_t * lock, rcutils_allocator_t allocator) +{ + (void)lock; + (void)allocator; + return RCUTILS_RET_OK; +} + +rcutils_ret_t +rcutils_rwlock_read_lock(rcutils_rwlock_t * lock) +{ + (void)lock; + return RCUTILS_RET_OK; +} + +rcutils_ret_t +rcutils_rwlock_read_unlock(rcutils_rwlock_t * lock) +{ + (void)lock; + return RCUTILS_RET_OK; +} + +rcutils_ret_t +rcutils_rwlock_write_lock(rcutils_rwlock_t * lock) +{ + (void)lock; + return RCUTILS_RET_OK; +} + +rcutils_ret_t +rcutils_rwlock_write_trylock(rcutils_rwlock_t * lock) +{ + (void)lock; + return RCUTILS_RET_OK; +} + +rcutils_ret_t +rcutils_rwlock_write_unlock(rcutils_rwlock_t * lock) +{ + (void)lock; + return RCUTILS_RET_OK; +} + +rcutils_ret_t +rcutils_rwlock_fini(rcutils_rwlock_t * lock) +{ + (void)lock; + return RCUTILS_RET_OK; +} diff --git a/src/rwlock.c b/src/rwlock_win32.c similarity index 58% rename from src/rwlock.c rename to src/rwlock_win32.c index 0970adcc..4f95a1b6 100644 --- a/src/rwlock.c +++ b/src/rwlock_win32.c @@ -12,123 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "./rwlock.h" // NOLINT - -#include "rcutils/allocator.h" -#include "rcutils/error_handling.h" -#include "rcutils/types/rcutils_ret.h" - -rcutils_rwlock_t -rcutils_get_zero_initialized_rwlock(void) -{ - static rcutils_rwlock_t zero_initialized_rwlock; - zero_initialized_rwlock.impl = NULL; - return zero_initialized_rwlock; -} - -#ifndef _WIN32 - -#include -#include - -typedef struct rcutils_rwlock_impl_s -{ - pthread_rwlock_t lock; - rcutils_allocator_t allocator; -} rcutils_rwlock_impl_t; - -rcutils_ret_t -rcutils_rwlock_init(rcutils_rwlock_t * lock, rcutils_allocator_t allocator) -{ - RCUTILS_CHECK_ARGUMENT_FOR_NULL(lock, RCUTILS_RET_INVALID_ARGUMENT); - if (lock->impl != NULL) { - RCUTILS_SET_ERROR_MSG("rwlock already initialized"); - return RCUTILS_RET_ERROR; - } - RCUTILS_CHECK_ALLOCATOR_WITH_MSG( - &allocator, "invalid allocator", return RCUTILS_RET_INVALID_ARGUMENT); - - lock->impl = allocator.allocate(sizeof(rcutils_rwlock_impl_t), allocator.state); - if (NULL == lock->impl) { - RCUTILS_SET_ERROR_MSG("failed to allocate memory for string map impl struct"); - return RCUTILS_RET_BAD_ALLOC; - } - - lock->impl->allocator = allocator; - - return pthread_rwlock_init(&lock->impl->lock, NULL) == 0 ? RCUTILS_RET_OK : RCUTILS_RET_ERROR; -} - -rcutils_ret_t -rcutils_rwlock_read_lock(rcutils_rwlock_t * lock) -{ - RCUTILS_CHECK_ARGUMENT_FOR_NULL(lock, RCUTILS_RET_INVALID_ARGUMENT); - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - lock->impl, "invalid lock", return RCUTILS_RET_ERROR); - - return pthread_rwlock_rdlock(&lock->impl->lock) == 0 ? RCUTILS_RET_OK : RCUTILS_RET_ERROR; -} - -rcutils_ret_t -rcutils_rwlock_read_unlock(rcutils_rwlock_t * lock) -{ - RCUTILS_CHECK_ARGUMENT_FOR_NULL(lock, RCUTILS_RET_INVALID_ARGUMENT); - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - lock->impl, "invalid lock", return RCUTILS_RET_ERROR); - - return pthread_rwlock_unlock(&lock->impl->lock) == 0 ? RCUTILS_RET_OK : RCUTILS_RET_ERROR; -} - -rcutils_ret_t -rcutils_rwlock_write_trylock(rcutils_rwlock_t * lock) -{ - RCUTILS_CHECK_ARGUMENT_FOR_NULL(lock, RCUTILS_RET_INVALID_ARGUMENT); - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - lock->impl, "invalid lock", return RCUTILS_RET_ERROR); - - return pthread_rwlock_trywrlock(&lock->impl->lock) == 0 ? RCUTILS_RET_OK : RCUTILS_RET_ERROR; -} - -rcutils_ret_t -rcutils_rwlock_write_lock(rcutils_rwlock_t * lock) -{ - RCUTILS_CHECK_ARGUMENT_FOR_NULL(lock, RCUTILS_RET_INVALID_ARGUMENT); - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - lock->impl, "invalid lock", return RCUTILS_RET_ERROR); - - return pthread_rwlock_wrlock(&lock->impl->lock) == 0 ? RCUTILS_RET_OK : RCUTILS_RET_ERROR; -} - -rcutils_ret_t -rcutils_rwlock_write_unlock(rcutils_rwlock_t * lock) -{ - RCUTILS_CHECK_ARGUMENT_FOR_NULL(lock, RCUTILS_RET_INVALID_ARGUMENT); - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - lock->impl, "invalid lock", return RCUTILS_RET_ERROR); - - return pthread_rwlock_unlock(&lock->impl->lock) == 0 ? RCUTILS_RET_OK : RCUTILS_RET_ERROR; -} - -rcutils_ret_t -rcutils_rwlock_fini(rcutils_rwlock_t * lock) -{ - RCUTILS_CHECK_ARGUMENT_FOR_NULL(lock, RCUTILS_RET_INVALID_ARGUMENT); - if (NULL == lock->impl) { - return RCUTILS_RET_OK; - } - - int retval = pthread_rwlock_destroy(&lock->impl->lock); - - rcutils_allocator_t allocator = lock->impl->allocator; - - allocator.deallocate(lock->impl, allocator.state); - lock->impl = NULL; - - return retval == 0 ? RCUTILS_RET_OK : RCUTILS_RET_ERROR; -} - -#else - // When building with MSVC 19.28.29333.0 on Windows 10 (as of 2020-11-11), // there appears to be a problem with winbase.h (which is included by // Windows.h). In particular, warnings of the form: @@ -142,12 +25,26 @@ rcutils_rwlock_fini(rcutils_rwlock_t * lock) #include #pragma warning(pop) +#include "./rwlock.h" // NOLINT + +#include "rcutils/allocator.h" +#include "rcutils/error_handling.h" +#include "rcutils/types/rcutils_ret.h" + typedef struct rcutils_rwlock_impl_s { SRWLOCK lock; rcutils_allocator_t allocator; } rcutils_rwlock_impl_t; +rcutils_rwlock_t +rcutils_get_zero_initialized_rwlock(void) +{ + static rcutils_rwlock_t zero_initialized_rwlock; + zero_initialized_rwlock.impl = NULL; + return zero_initialized_rwlock; +} + rcutils_ret_t rcutils_rwlock_init(rcutils_rwlock_t * lock, rcutils_allocator_t allocator) { @@ -246,5 +143,3 @@ rcutils_rwlock_fini(rcutils_rwlock_t * lock) return RCUTILS_RET_OK; } - -#endif