Title: [287994] trunk/Source/bmalloc
- Revision
- 287994
- Author
- fpi...@apple.com
- Date
- 2022-01-13 14:45:11 -0800 (Thu, 13 Jan 2022)
Log Message
[libpas] pas_segregated_page_lock_with_mode in try_lock mode should check that the page still uses the lock after the try_lock
https://bugs.webkit.org/show_bug.cgi?id=235203
Reviewed by Yusuke Suzuki.
The bug I was trying to find by assertions in bug 235190 is that lock_with_mode has an incorrect
implementation of the try_lock case. It forgets to check if the lock it acquired is the right one
after locking.
I don't know how to test this without writing a test that is very gross. It's a super subtle race
condition - one that would be hard to reliably trigger even if I used the race_test_hooks
functionality.
* libpas/src/libpas/pas_local_allocator.c:
(stop_impl):
* libpas/src/libpas/pas_segregated_page_inlines.h:
(pas_segregated_page_switch_lock_with_mode):
Modified Paths
Diff
Modified: trunk/Source/bmalloc/ChangeLog (287993 => 287994)
--- trunk/Source/bmalloc/ChangeLog 2022-01-13 21:53:09 UTC (rev 287993)
+++ trunk/Source/bmalloc/ChangeLog 2022-01-13 22:45:11 UTC (rev 287994)
@@ -1,5 +1,25 @@
2022-01-13 Filip Pizlo <fpi...@apple.com>
+ [libpas] pas_segregated_page_lock_with_mode in try_lock mode should check that the page still uses the lock after the try_lock
+ https://bugs.webkit.org/show_bug.cgi?id=235203
+
+ Reviewed by Yusuke Suzuki.
+
+ The bug I was trying to find by assertions in bug 235190 is that lock_with_mode has an incorrect
+ implementation of the try_lock case. It forgets to check if the lock it acquired is the right one
+ after locking.
+
+ I don't know how to test this without writing a test that is very gross. It's a super subtle race
+ condition - one that would be hard to reliably trigger even if I used the race_test_hooks
+ functionality.
+
+ * libpas/src/libpas/pas_local_allocator.c:
+ (stop_impl):
+ * libpas/src/libpas/pas_segregated_page_inlines.h:
+ (pas_segregated_page_switch_lock_with_mode):
+
+2022-01-13 Filip Pizlo <fpi...@apple.com>
+
[libpas] add assertions that we aren't switching to a NULL lock
https://bugs.webkit.org/show_bug.cgi?id=235190
Modified: trunk/Source/bmalloc/libpas/src/libpas/pas_local_allocator.c (287993 => 287994)
--- trunk/Source/bmalloc/libpas/src/libpas/pas_local_allocator.c 2022-01-13 21:53:09 UTC (rev 287993)
+++ trunk/Source/bmalloc/libpas/src/libpas/pas_local_allocator.c 2022-01-13 22:45:11 UTC (rev 287994)
@@ -181,8 +181,8 @@
if (!pas_segregated_page_switch_lock_with_mode(page, &held_lock, page_lock_mode, page_config))
return false;
- if (!pas_segregated_page_config_is_utility(page_config) && !held_lock)
- pas_panic("Should be holding a lock after pas_segregated_page_switch_lock_with_mode in stop_impl\n");
+ if (!pas_segregated_page_config_is_utility(page_config))
+ PAS_ASSERT(held_lock);
page_config.specialized_local_allocator_return_memory_to_page(
allocator, view, page, directory, heap_lock_hold_mode);
Modified: trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_page_inlines.h (287993 => 287994)
--- trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_page_inlines.h 2022-01-13 21:53:09 UTC (rev 287993)
+++ trunk/Source/bmalloc/libpas/src/libpas/pas_segregated_page_inlines.h 2022-01-13 22:45:11 UTC (rev 287994)
@@ -240,9 +240,33 @@
switch (lock_mode) {
case pas_lock_lock_mode_try_lock: {
pas_lock* page_lock;
+ pas_compiler_fence();
page_lock = page->lock_ptr;
PAS_TESTING_ASSERT(page_lock);
- return pas_lock_switch_with_mode(held_lock, page_lock, pas_lock_lock_mode_try_lock);
+ if (*held_lock == page_lock) {
+ pas_compiler_fence();
+ return true;
+ }
+ pas_compiler_fence();
+ if (*held_lock)
+ pas_lock_unlock(*held_lock);
+ pas_compiler_fence();
+ for (;;) {
+ pas_lock* new_page_lock;
+ if (!pas_lock_try_lock(page_lock)) {
+ *held_lock = NULL;
+ pas_compiler_fence();
+ return false;
+ }
+ new_page_lock = page->lock_ptr;
+ if (page_lock == new_page_lock) {
+ *held_lock = page_lock;
+ pas_compiler_fence();
+ return true;
+ }
+ pas_lock_unlock(page_lock);
+ page_lock = new_page_lock;
+ }
}
case pas_lock_lock_mode_lock: {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
