Skip to content

Commit

Permalink
Merge pull request #2743 from particle-iot/fix/std-once-reentrancy
Browse files Browse the repository at this point in the history
Fixes std::call_once reentrancy problem
  • Loading branch information
avtolstoy authored Feb 22, 2024
2 parents a72f111 + 52d9b70 commit ac77bcd
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 173 deletions.
23 changes: 0 additions & 23 deletions hal/src/nRF52840/bits/gthr.cpp

This file was deleted.

1 change: 1 addition & 0 deletions hal/src/nRF52840/include.mk
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ HAL_SRC_INCL_PATH = $(TARGET_HAL_PATH)/src/nRF52840

INCLUDE_DIRS += $(HAL_INCL_NRF52840_PATH)
INCLUDE_DIRS += $(HAL_INCL_ARMV7_PATH)
INCLUDE_DIRS += $(TARGET_HAL_PATH)/src/portable/FreeRTOS

ifneq (,$(findstring platform,$(DEPENDENCIES)))
INCLUDE_DIRS += $(HAL_INCL_NRF52840_PATH)
Expand Down
1 change: 1 addition & 0 deletions hal/src/nRF52840/sources.mk
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ CPPSRC += $(call target_files,$(HAL_MODULE_PATH)/shared/,*.cpp)
CPPSRC += $(call target_files,$(HAL_MODULE_PATH)/network/util/,*.cpp)

CSRC += $(TARGET_HAL_PATH)/src/portable/FreeRTOS/heap_4_lock.c
CPPSRC += $(call target_files,$(TARGET_HAL_PATH)/src/portable/FreeRTOS/bits/,*.cpp)

# ASM source files included in this build.
ASRC +=
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,14 @@
#include <atomic>

typedef int __gthread_key_t;
typedef std::atomic_flag __gthread_once_t;

typedef enum __gthread_once_state_t {
GTHREAD_ONCE_STATE_NOT_INITIALIZED = 0,
GTHREAD_ONCE_STATE_RUNNING = 1,
GTHREAD_ONCE_STATE_INITIALIZED = 2
} __gthread_once_state_t;

typedef std::atomic<__gthread_once_state_t> __gthread_once_t;

#define _GLIBCXX_UNUSED __attribute__((unused))

Expand All @@ -25,14 +31,7 @@ __gthread_active_p (void)
return 0;
}

static inline int
__gthread_once (__gthread_once_t* once, void (*func) (void))
{
if (once->test_and_set(std::memory_order_relaxed) == false) {
func();
}
return 0;
}
extern "C" int __gthread_once (__gthread_once_t* once, void (*func) (void));

static inline int _GLIBCXX_UNUSED
__gthread_key_create (__gthread_key_t *__key _GLIBCXX_UNUSED, void (*__func) (void *) _GLIBCXX_UNUSED)
Expand Down Expand Up @@ -106,7 +105,7 @@ __gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex)
return os_mutex_recursive_destroy(*__mutex);
}

#define __GTHREAD_ONCE_INIT ATOMIC_FLAG_INIT
#define __GTHREAD_ONCE_INIT ATOMIC_VAR_INIT(GTHREAD_ONCE_STATE_NOT_INITIALIZED)
#define __GTHREAD_MUTEX_INIT_FUNCTION(mx) os_mutex_create(mx)
#define __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION(mx) os_mutex_recursive_create(mx)

Expand Down
68 changes: 68 additions & 0 deletions hal/src/portable/FreeRTOS/bits/gthr.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright (c) 2018 Particle Industries, Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation, either
* version 3 of the License, or (at your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/

#include <bits/gthr.h>
#include <mutex>
#include "scope_guard.h"
#include "gthr-default.h"
#include "delay_hal.h"

extern "C" void __once_proxy(void) {
std::__once_functor();
}

namespace std {

extern std::unique_lock<std::mutex>*& __get_once_functor_lock_ptr();

} // std

// XXX/FIXME: std::call_once platform-specific implementations are not exported to the application
// so we should probably just get rid of std::call_once altogether and replace with a separate implememntation.
extern "C" int __gthread_once (__gthread_once_t* once, void (*func) (void))
{
if (once->load(std::memory_order_acquire) == GTHREAD_ONCE_STATE_INITIALIZED) {
// Short path, already initialized
return 0;
}

// Make a local copy, this is a workaround for a non-entrant implementation in libstdc++
auto callable = std::__once_functor;
auto lock = std::__get_once_functor_lock_ptr();
if (lock) {
// For consistency
std::__set_once_functor_lock_ptr(0);
// This won't be relocked in <mutex> std::call_once implementation
lock->unlock();
}
// std::__once_functor() is not usable from this point on, local callable should be used

auto state = GTHREAD_ONCE_STATE_NOT_INITIALIZED;
if (once->compare_exchange_strong(state, GTHREAD_ONCE_STATE_RUNNING,
std::memory_order_acq_rel)) {
// Switched to initializing/running state
callable();
once->store(GTHREAD_ONCE_STATE_INITIALIZED, std::memory_order_release);
} else {
// Wait for some other thread to complete initialization
while (once->load(std::memory_order_acquire) == GTHREAD_ONCE_STATE_RUNNING) {
HAL_Delay_Milliseconds(0);
}
}

return 0;
}
115 changes: 0 additions & 115 deletions hal/src/rtl872x/bits/gthr-default.h

This file was deleted.

23 changes: 0 additions & 23 deletions hal/src/rtl872x/bits/gthr.cpp

This file was deleted.

1 change: 1 addition & 0 deletions hal/src/rtl872x/include.mk
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ HAL_INCL_RTL872X_PATH = $(TARGET_HAL_PATH)/src/rtl872x
HAL_SRC_INCL_PATH = $(TARGET_HAL_PATH)/src/rtl872x

INCLUDE_DIRS += $(HAL_INCL_RTL872X_PATH)
INCLUDE_DIRS += $(TARGET_HAL_PATH)/src/portable/FreeRTOS

ifneq (,$(findstring platform,$(DEPENDENCIES)))
INCLUDE_DIRS += $(HAL_INCL_RTL872X_PATH)
Expand Down
2 changes: 1 addition & 1 deletion hal/src/rtl872x/sources.mk
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ CPPSRC += $(call target_files,$(HAL_MODULE_PATH)/shared/,*.cpp)
CPPSRC += $(call target_files,$(HAL_MODULE_PATH)/network/util/,*.cpp)

CSRC += $(TARGET_HAL_PATH)/src/portable/FreeRTOS/heap_5_lock.c
INCLUDE_DIRS += $(TARGET_HAL_PATH)/src/portable/FreeRTOS
CPPSRC += $(call target_files,$(TARGET_HAL_PATH)/src/portable/FreeRTOS/bits/,*.cpp)

# ASM source files included in this build.
ASRC +=
2 changes: 1 addition & 1 deletion system/src/system_threading.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ namespace std {
return __once;
}

inline std::unique_lock<std::mutex>*& __get_once_functor_lock_ptr()
std::unique_lock<std::mutex>*& __get_once_functor_lock_ptr()
{
static std::unique_lock<std::mutex>* __once_functor_lock_ptr = 0;
return __once_functor_lock_ptr;
Expand Down

0 comments on commit ac77bcd

Please sign in to comment.