- Revision
- 286573
- Author
- [email protected]
- Date
- 2021-12-06 14:53:46 -0800 (Mon, 06 Dec 2021)
Log Message
[libpas] Set pthread_setspecific with marker in TLS destructor to detect TLS is destroyed
https://bugs.webkit.org/show_bug.cgi?id=233851
Reviewed by Mark Lam.
TLS has lifetime problem that,
1. TLS is destroyed
2. The other TLS is destroyed, and the destructor is called
3. The destructor touches (1)'s TLS, then revive TLS of (1) (e.g. libpas's thread local cache).
To handle these cases, pthread library (libc) repeatedly calls destructor PAS_THREAD_LOCAL_CACHE_DESTROYED times
so that we clean up revived TLS again.
By using this mechanism, we can emulate pthread_self_is_exiting_np so that we can avoid reviving TLS.
1. When destroying TLS, we set a marker (PAS_THREAD_LOCAL_CACHE_DESTROYED in this case) in TLS.
2. During the other destructor calls, we can detect that TLS is destroyed by checking pthread_getspecific(...) == PAS_THREAD_LOCAL_CACHE_DESTROYED.
3. We repeatedly calls the destructor of TLS, but every time, we set PAS_THREAD_LOCAL_CACHE_DESTROYED.
So after PAS_THREAD_LOCAL_CACHE_DESTROYED times, it is left, and we achieve the goal (1) offering the way to
detect the destroyed TLS and (2) avoiding reviving of TLS.
This patch implements it when pthread_self_is_exiting_np does not exist.
* libpas/src/libpas/pas_segregated_page_inlines.h:
(pas_segregated_page_switch_lock_with_mode):
* libpas/src/libpas/pas_thread_local_cache.c:
(destructor):
(pas_thread_local_cache_create):
(pas_thread_local_cache_destroy):
(pas_thread_local_cache_get_local_allocator_slow):
(pas_thread_local_cache_for_all):
* libpas/src/libpas/pas_thread_local_cache.h:
(pas_thread_local_cache_try_get_impl):
(pas_thread_local_cache_try_get):
(pas_thread_local_cache_can_set):
(pas_thread_local_cache_set_impl):
(pas_thread_local_cache_set):
(pas_thread_local_cache_is_guaranteed_to_destruct): Deleted.
* libpas/src/libpas/pas_try_reallocate.h:
(pas_try_reallocate):
* libpas/src/libpas/pas_utils.h:
* libpas/src/test/IsoHeapPageSharingTests.cpp:
(std::addAllTests):
* libpas/src/test/IsoHeapPartialAndBaselineTests.cpp:
(addIsoHeapPartialAndBaselineTests):
Modified Paths
Diff
Modified: trunk/Source/bmalloc/ChangeLog (286572 => 286573)
--- trunk/Source/bmalloc/ChangeLog 2021-12-06 22:50:39 UTC (rev 286572)
+++ trunk/Source/bmalloc/ChangeLog 2021-12-06 22:53:46 UTC (rev 286573)
@@ -1,3 +1,52 @@
+2021-12-06 Yusuke Suzuki <[email protected]>
+
+ [libpas] Set pthread_setspecific with marker in TLS destructor to detect TLS is destroyed
+ https://bugs.webkit.org/show_bug.cgi?id=233851
+
+ Reviewed by Mark Lam.
+
+ TLS has lifetime problem that,
+
+ 1. TLS is destroyed
+ 2. The other TLS is destroyed, and the destructor is called
+ 3. The destructor touches (1)'s TLS, then revive TLS of (1) (e.g. libpas's thread local cache).
+
+ To handle these cases, pthread library (libc) repeatedly calls destructor PAS_THREAD_LOCAL_CACHE_DESTROYED times
+ so that we clean up revived TLS again.
+
+ By using this mechanism, we can emulate pthread_self_is_exiting_np so that we can avoid reviving TLS.
+
+ 1. When destroying TLS, we set a marker (PAS_THREAD_LOCAL_CACHE_DESTROYED in this case) in TLS.
+ 2. During the other destructor calls, we can detect that TLS is destroyed by checking pthread_getspecific(...) == PAS_THREAD_LOCAL_CACHE_DESTROYED.
+ 3. We repeatedly calls the destructor of TLS, but every time, we set PAS_THREAD_LOCAL_CACHE_DESTROYED.
+ So after PAS_THREAD_LOCAL_CACHE_DESTROYED times, it is left, and we achieve the goal (1) offering the way to
+ detect the destroyed TLS and (2) avoiding reviving of TLS.
+
+ This patch implements it when pthread_self_is_exiting_np does not exist.
+
+ * libpas/src/libpas/pas_segregated_page_inlines.h:
+ (pas_segregated_page_switch_lock_with_mode):
+ * libpas/src/libpas/pas_thread_local_cache.c:
+ (destructor):
+ (pas_thread_local_cache_create):
+ (pas_thread_local_cache_destroy):
+ (pas_thread_local_cache_get_local_allocator_slow):
+ (pas_thread_local_cache_for_all):
+ * libpas/src/libpas/pas_thread_local_cache.h:
+ (pas_thread_local_cache_try_get_impl):
+ (pas_thread_local_cache_try_get):
+ (pas_thread_local_cache_can_set):
+ (pas_thread_local_cache_set_impl):
+ (pas_thread_local_cache_set):
+ (pas_thread_local_cache_is_guaranteed_to_destruct): Deleted.
+ * libpas/src/libpas/pas_try_reallocate.h:
+ (pas_try_reallocate):
+ * libpas/src/libpas/pas_utils.h:
+ * libpas/src/test/IsoHeapPageSharingTests.cpp:
+ (std::addAllTests):
+ * libpas/src/test/IsoHeapPartialAndBaselineTests.cpp:
+ (addIsoHeapPartialAndBaselineTests):
+
2021-12-03 Filip Pizlo <[email protected]>
[libpas] Bitfit allocator has a wrong assertion when a page's max_free is enough for the size of an allocation, not enough for that allocation's size class, and the object of that size is not aligned to the currently requested alignment
Modified: trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_page_inlines.h (286572 => 286573)
--- trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_page_inlines.h 2021-12-06 22:50:39 UTC (rev 286572)
+++ trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_page_inlines.h 2021-12-06 22:53:46 UTC (rev 286573)
@@ -278,6 +278,7 @@
return true;
} }
PAS_ASSERT(!"Should not be reached");
+ return true;
}
static PAS_ALWAYS_INLINE void pas_segregated_page_switch_lock(
Modified: trunk/Source/bmalloc/libpas/src/libpas/pas_thread_local_cache.c (286572 => 286573)
--- trunk/Source/bmalloc/libpas/src/libpas/pas_thread_local_cache.c 2021-12-06 22:50:39 UTC (rev 286572)
+++ trunk/Source/bmalloc/libpas/src/libpas/pas_thread_local_cache.c 2021-12-06 22:53:46 UTC (rev 286573)
@@ -91,11 +91,28 @@
static void destructor(void* arg)
{
+ static const bool verbose = false;
+
pas_thread_local_cache* thread_local_cache;
thread_local_cache = (pas_thread_local_cache*)arg;
- destroy(thread_local_cache, pas_lock_is_not_held);
+#ifndef PAS_THREAD_LOCAL_CACHE_CAN_DETECT_THREAD_EXIT
+ /* If pthread_self_is_exiting_np does not exist, we set PAS_THREAD_LOCAL_CACHE_DESTROYED in the TLS so that
+ subsequent calls of pas_thread_local_cache_try_get() can detect whether TLS is destroyed. Since
+ PAS_THREAD_LOCAL_CACHE_DESTROYED is a non-null value, pthread will call this destructor again (up to
+ PTHREAD_DESTRUCTOR_ITERATIONS times). Each time it does, it will clear the TLS entry. Hence, we need to re-set
+ PAS_THREAD_LOCAL_CACHE_DESTROYED in the TLS each time to continue to indicate that destroy() has already been called once. */
+ pas_thread_local_cache_set_impl((pas_thread_local_cache*)PAS_THREAD_LOCAL_CACHE_DESTROYED);
+ PAS_ASSERT(!pas_thread_local_cache_can_set());
+#endif
+
+ if (((uintptr_t)thread_local_cache) != PAS_THREAD_LOCAL_CACHE_DESTROYED)
+ destroy(thread_local_cache, pas_lock_is_not_held);
+ else {
+ if (verbose)
+ pas_log("[%d] Repeated destructor call for TLS %p\n", getpid(), thread_local_cache);
+ }
}
static pas_thread_local_cache* allocate_cache(unsigned allocator_index_capacity)
@@ -174,7 +191,7 @@
for (PAS_THREAD_LOCAL_CACHE_LAYOUT_EACH_ALLOCATOR(layout_node))
pas_thread_local_cache_layout_node_construct(layout_node, thread_local_cache);
- pas_thread_local_cache_set_impl(thread_local_cache);
+ pas_thread_local_cache_set(thread_local_cache);
return thread_local_cache;
}
@@ -194,7 +211,7 @@
pas_log("[%d] TLC %p getting destroyed\n", getpid(), thread_local_cache);
destroy(thread_local_cache, heap_lock_hold_mode);
- pas_thread_local_cache_set_impl(NULL);
+ pas_thread_local_cache_set(NULL);
}
pas_thread_local_cache* pas_thread_local_cache_get_slow(pas_heap_config* config,
@@ -312,7 +329,7 @@
pas_heap_lock_unlock_conditionally(heap_lock_hold_mode);
if (thread_local_cache != new_thread_local_cache)
- pas_thread_local_cache_set_impl(new_thread_local_cache);
+ pas_thread_local_cache_set(new_thread_local_cache);
PAS_ASSERT(desired_allocator_index < new_thread_local_cache->allocator_index_upper_bound);
return pas_local_allocator_result_create_success(
@@ -566,7 +583,7 @@
pas_scavenger_notify_eligibility_if_needed();
}
-#ifdef PAS_THREAD_LOCAL_CACHE_CAN_DETECT_THREAD_EXIT
+#if PAS_OS(DARWIN)
static void suspend(pas_thread_local_cache* cache)
{
@@ -740,14 +757,7 @@
continue;
}
-#ifdef PAS_THREAD_LOCAL_CACHE_CAN_DETECT_THREAD_EXIT
- if (!pas_thread_local_cache_is_guaranteed_to_destruct()) {
- /* We're on a platform that can't guarantee that thread local caches are destructed.
- Therefore, we might have a TLC that has a dangling thread pointer. So, we don't
- attempt to do the suspend thing. */
- continue;
- }
-
+#if PAS_OS(DARWIN)
if (verbose)
pas_log("Need to suspend for allocator %p\n", scavenger_data);
@@ -769,7 +779,7 @@
#endif
}
-#ifdef PAS_THREAD_LOCAL_CACHE_CAN_DETECT_THREAD_EXIT
+#if PAS_OS(DARWIN)
if (did_suspend)
resume(cache);
#endif
Modified: trunk/Source/bmalloc/libpas/src/libpas/pas_thread_local_cache.h (286572 => 286573)
--- trunk/Source/bmalloc/libpas/src/libpas/pas_thread_local_cache.h 2021-12-06 22:50:39 UTC (rev 286572)
+++ trunk/Source/bmalloc/libpas/src/libpas/pas_thread_local_cache.h 2021-12-06 22:53:46 UTC (rev 286573)
@@ -44,6 +44,18 @@
#define PAS_HAVE_PTHREAD_PRIVATE 0
#endif
+#define PAS_THREAD_LOCAL_CACHE_DESTROYED 1
+
+#if PAS_HAVE_PTHREAD_PRIVATE
+#if (PAS_PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 110000) \
+ || (PAS_PLATFORM(MACCATALYST) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000) \
+ || (PAS_PLATFORM(IOS) && PAS_PLATFORM(IOS_FAMILY_SIMULATOR) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000) \
+ || (PAS_PLATFORM(WATCHOS) && PAS_PLATFORM(IOS_FAMILY_SIMULATOR) && __WATCH_OS_VERSION_MIN_REQUIRED >= 70000) \
+ || (PAS_PLATFORM(APPLETV) && PAS_PLATFORM(IOS_FAMILY_SIMULATOR) && __TV_OS_VERSION_MIN_REQUIRED >= 140000)
+#define PAS_THREAD_LOCAL_CACHE_CAN_DETECT_THREAD_EXIT 1
+#endif
+#endif
+
PAS_BEGIN_EXTERN_C;
struct pas_magazine;
@@ -83,28 +95,19 @@
#define PAS_THREAD_LOCAL_KEY __PTK_FRAMEWORK_JAVASCRIPTCORE_KEY4
-static inline pas_thread_local_cache* pas_thread_local_cache_try_get(void)
+static PAS_ALWAYS_INLINE pas_thread_local_cache* pas_thread_local_cache_try_get_impl(void)
{
- return (pas_thread_local_cache*)PAS_FAST_TLS_GET(
- PAS_THREAD_LOCAL_KEY, &pas_thread_local_cache_fast_tls);
+ return (pas_thread_local_cache*)PAS_FAST_TLS_GET(PAS_THREAD_LOCAL_KEY, &pas_thread_local_cache_fast_tls);
}
-#if PAS_HAVE_PTHREAD_PRIVATE
-#if (PAS_PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 110000) \
- || (PAS_PLATFORM(MACCATALYST) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000) \
- || (PAS_PLATFORM(IOS) && PAS_PLATFORM(IOS_FAMILY_SIMULATOR) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000) \
- || (PAS_PLATFORM(WATCHOS) && PAS_PLATFORM(IOS_FAMILY_SIMULATOR) && __WATCH_OS_VERSION_MIN_REQUIRED >= 70000) \
- || (PAS_PLATFORM(APPLETV) && PAS_PLATFORM(IOS_FAMILY_SIMULATOR) && __TV_OS_VERSION_MIN_REQUIRED >= 140000)
-#define PAS_THREAD_LOCAL_CACHE_CAN_DETECT_THREAD_EXIT 1
-#endif
-
-static inline bool pas_thread_local_cache_is_guaranteed_to_destruct(void)
+static inline pas_thread_local_cache* pas_thread_local_cache_try_get(void)
{
-#ifdef PAS_THREAD_LOCAL_CACHE_CAN_DETECT_THREAD_EXIT
- return true;
-#else
- return false;
+ pas_thread_local_cache* cache = pas_thread_local_cache_try_get_impl();
+#ifndef PAS_THREAD_LOCAL_CACHE_CAN_DETECT_THREAD_EXIT
+ if (((uintptr_t)cache) == PAS_THREAD_LOCAL_CACHE_DESTROYED)
+ return NULL;
#endif
+ return cache;
}
static inline bool pas_thread_local_cache_can_set(void)
@@ -112,25 +115,19 @@
#ifdef PAS_THREAD_LOCAL_CACHE_CAN_DETECT_THREAD_EXIT
return !pthread_self_is_exiting_np();
#else
- return true;
+ return ((uintptr_t)pas_thread_local_cache_try_get_impl()) != PAS_THREAD_LOCAL_CACHE_DESTROYED;
#endif
}
-#else /* PAS_HAVE_PTHREAD_PRIVATE -> so !PAS_HAVE_PTHREAD_PRIVATE */
-static inline bool pas_thread_local_cache_is_guaranteed_to_destruct(void)
-{
- return false;
-}
-static inline bool pas_thread_local_cache_can_set(void)
+static inline void pas_thread_local_cache_set_impl(pas_thread_local_cache* thread_local_cache)
{
- return true;
+ PAS_FAST_TLS_SET(PAS_THREAD_LOCAL_KEY, &pas_thread_local_cache_fast_tls, thread_local_cache);
}
-#endif /* PAS_HAVE_PTHREAD_PRIVATE -> so end of !PAS_HAVE_PTHREAD_PRIVATE */
-static inline void pas_thread_local_cache_set_impl(pas_thread_local_cache* thread_local_cache)
+static inline void pas_thread_local_cache_set(pas_thread_local_cache* thread_local_cache)
{
PAS_ASSERT(pas_thread_local_cache_can_set() || pas_thread_local_cache_try_get());
- PAS_FAST_TLS_SET(PAS_THREAD_LOCAL_KEY, &pas_thread_local_cache_fast_tls, thread_local_cache);
+ pas_thread_local_cache_set_impl(thread_local_cache);
}
PAS_API size_t pas_thread_local_cache_size_for_allocator_index_capacity(unsigned allocator_index_capacity);
Modified: trunk/Source/bmalloc/libpas/src/libpas/pas_try_reallocate.h (286572 => 286573)
--- trunk/Source/bmalloc/libpas/src/libpas/pas_try_reallocate.h 2021-12-06 22:50:39 UTC (rev 286572)
+++ trunk/Source/bmalloc/libpas/src/libpas/pas_try_reallocate.h 2021-12-06 22:53:46 UTC (rev 286573)
@@ -354,6 +354,7 @@
} }
PAS_ASSERT(!"Should never be reached");
+ return pas_allocation_result_create_failure();
}
typedef struct {
Modified: trunk/Source/bmalloc/libpas/src/libpas/pas_utils.h (286572 => 286573)
--- trunk/Source/bmalloc/libpas/src/libpas/pas_utils.h 2021-12-06 22:50:39 UTC (rev 286572)
+++ trunk/Source/bmalloc/libpas/src/libpas/pas_utils.h 2021-12-06 22:53:46 UTC (rev 286573)
@@ -108,14 +108,13 @@
PAS_API PAS_NO_RETURN void pas_assertion_failed(const char* filename, int line, const char* function, const char* _expression_);
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wmissing-noreturn"
+PAS_IGNORE_WARNINGS_BEGIN("missing-noreturn")
static inline void pas_assertion_failed_noreturn_silencer(
const char* filename, int line, const char* function, const char* _expression_)
{
pas_assertion_failed(filename, line, function, _expression_);
}
-#pragma clang diagnostic pop
+PAS_IGNORE_WARNINGS_END
#define PAS_LIKELY(x) __PAS_LIKELY(x)
#define PAS_UNLIKELY(x) __PAS_UNLIKELY(x)
Modified: trunk/Source/bmalloc/libpas/src/test/IsoHeapPageSharingTests.cpp (286572 => 286573)
--- trunk/Source/bmalloc/libpas/src/test/IsoHeapPageSharingTests.cpp 2021-12-06 22:50:39 UTC (rev 286572)
+++ trunk/Source/bmalloc/libpas/src/test/IsoHeapPageSharingTests.cpp 2021-12-06 22:53:46 UTC (rev 286573)
@@ -4519,11 +4519,9 @@
ADD_TEST(testScavengerEventuallyReturnsMemory(128, 1));
ADD_TEST(testScavengerEventuallyReturnsMemory(128, 10000));
ADD_TEST(testScavengerEventuallyReturnsMemory(8, 10000));
- if (pas_thread_local_cache_is_guaranteed_to_destruct()) {
- ADD_TEST(testScavengerEventuallyReturnsMemoryEvenWithoutManualShrink(128, 1));
- ADD_TEST(testScavengerEventuallyReturnsMemoryEvenWithoutManualShrink(128, 10000));
- ADD_TEST(testScavengerEventuallyReturnsMemoryEvenWithoutManualShrink(8, 10000));
- }
+ ADD_TEST(testScavengerEventuallyReturnsMemoryEvenWithoutManualShrink(128, 1));
+ ADD_TEST(testScavengerEventuallyReturnsMemoryEvenWithoutManualShrink(128, 10000));
+ ADD_TEST(testScavengerEventuallyReturnsMemoryEvenWithoutManualShrink(8, 10000));
ADD_TEST(testScavengerShutsDownEventually(64, 10000, 1, 1));
}
@@ -4530,11 +4528,9 @@
ADD_TEST(testScavengerEventuallyReturnsMemory(128, 1));
ADD_TEST(testScavengerEventuallyReturnsMemory(128, 10000));
ADD_TEST(testScavengerEventuallyReturnsMemory(8, 10000));
- if (pas_thread_local_cache_is_guaranteed_to_destruct()) {
- ADD_TEST(testScavengerEventuallyReturnsMemoryEvenWithoutManualShrink(128, 1));
- ADD_TEST(testScavengerEventuallyReturnsMemoryEvenWithoutManualShrink(128, 10000));
- ADD_TEST(testScavengerEventuallyReturnsMemoryEvenWithoutManualShrink(8, 10000));
- }
+ ADD_TEST(testScavengerEventuallyReturnsMemoryEvenWithoutManualShrink(128, 1));
+ ADD_TEST(testScavengerEventuallyReturnsMemoryEvenWithoutManualShrink(128, 10000));
+ ADD_TEST(testScavengerEventuallyReturnsMemoryEvenWithoutManualShrink(8, 10000));
ADD_TEST(testScavengerShutsDownEventually(64, 10000, 1, 1));
}
Modified: trunk/Source/bmalloc/libpas/src/test/IsoHeapPartialAndBaselineTests.cpp (286572 => 286573)
--- trunk/Source/bmalloc/libpas/src/test/IsoHeapPartialAndBaselineTests.cpp 2021-12-06 22:50:39 UTC (rev 286572)
+++ trunk/Source/bmalloc/libpas/src/test/IsoHeapPartialAndBaselineTests.cpp 2021-12-06 22:53:46 UTC (rev 286573)
@@ -1082,7 +1082,7 @@
SuspendScavengerScope suspendScavenger;
#if PAS_ENABLE_ISO && PAS_ENABLE_ISO_TEST
- if (pas_thread_local_cache_is_guaranteed_to_destruct()) {
+ {
RunScavengerFully runScavengerFully;
addScavengerDependentTests();
}
