Skip to content

Commit 08cf7cf

Browse files
committed
Fix scanning of weak key hash tables
We have been finding intermittent memory-test failures after registering finalizers. The problem appears to be that the Buckets for the values, which should be strong references, were instead allocated boehm-atomically and therefore not scanned. This led to live objects being collected, which causes lots of weird problems. This only happened after registering finalizers because we use a weak key hash table for finalizers internally. The weak key hash table code is pretty brittle and I'm replacing it in another branch.
1 parent 2381c77 commit 08cf7cf

File tree

2 files changed

+18
-18
lines changed

2 files changed

+18
-18
lines changed

include/clasp/gctools/gcalloc.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ THE SOFTWARE.
3333
// #define DEBUG_CONS_ALLOC 1
3434

3535
#include <limits>
36+
#include <type_traits> // is_same_v
3637
#include <clasp/gctools/interrupt.h>
3738
#include <clasp/gctools/threadlocal.fwd.h>
3839
#include <clasp/gctools/snapshotSaveLoad.fwd.h>
@@ -239,8 +240,7 @@ template <class Stage, class Cons, class Register> struct ConsAllocator {
239240
#ifdef USE_PRECISE_GC
240241
static smart_ptr<Cons> snapshot_save_load_allocate(Header_s::BadgeStampWtagMtag& the_header, core::T_sp car, core::T_sp cdr) {
241242
#if defined(USE_BOEHM)
242-
Header_s* header = reinterpret_cast<Header_s*>(ALIGNED_GC_MALLOC_KIND(
243-
STAMP_UNSHIFT_WTAG(STAMPWTAG_CONS), SizeofConsHeader() + sizeof(Cons), global_cons_kind, &global_cons_kind)); // wasMTAG
243+
Header_s* header = reinterpret_cast<Header_s*>(ALIGNED_GC_MALLOC_KIND(SizeofConsHeader() + sizeof(Cons), global_cons_kind)); // wasMTAG
244244
header->_badge_stamp_wtag_mtag._header_badge.store(the_header._header_badge.load());
245245
header->_badge_stamp_wtag_mtag._value = the_header._value;
246246
#elif defined(USE_MMTK)
@@ -892,6 +892,7 @@ template <class VT, class StrongWeakLinkType> class GCBucketAllocator<Buckets<VT
892892
typedef const value_type& const_reference;
893893
typedef std::size_t size_type;
894894
typedef std::ptrdiff_t difference_type;
895+
static constexpr bool weakp = std::is_same_v<StrongWeakLinkType, WeakLinks>;
895896

896897
/* constructors and destructor
897898
* - nothing to do because the allocator has no state
@@ -906,7 +907,7 @@ template <class VT, class StrongWeakLinkType> class GCBucketAllocator<Buckets<VT
906907
// allocate but don't initialize num elements of type value_type
907908
static gctools::tagged_pointer<container_type> allocate(Header_s::BadgeStampWtagMtag the_header, size_type num, const void* = 0) {
908909
size_t size = sizeof_container_with_header<container_type>(num);
909-
Header_s* base = do_weak_allocation(the_header, size);
910+
Header_s* base = do_weak_allocation<weakp>(the_header, size);
910911
container_pointer myAddress = (container_pointer)HeaderPtrToWeakPtr(base);
911912
if (!myAddress)
912913
throw_hard_error("Out of memory in allocate");
@@ -919,7 +920,7 @@ template <class VT, class StrongWeakLinkType> class GCBucketAllocator<Buckets<VT
919920

920921
static gctools::tagged_pointer<container_type> snapshot_save_load_allocate(snapshotSaveLoad::snapshot_save_load_init_s* init) {
921922
size_t size = (init->_clientEnd - init->_clientStart) + SizeofWeakHeader();
922-
Header_s* base = do_weak_allocation(init->_headStart->_badge_stamp_wtag_mtag, size);
923+
Header_s* base = do_weak_allocation<weakp>(init->_headStart->_badge_stamp_wtag_mtag, size);
923924
#ifdef DEBUG_GUARD
924925
// Copy the source from the image save/load memory.
925926
base->_source = init->_headStart->_source;
@@ -971,7 +972,7 @@ template <class VT, class StrongWeakLinkType> class GCMappingAllocator<Mapping<V
971972
// allocate but don't initialize num elements of type value_type
972973
static gctools::tagged_pointer<container_type> allocate(Header_s::BadgeStampWtagMtag the_header, const VT& val) {
973974
size_t size = sizeof_with_header<container_type>();
974-
Header_s* base = do_weak_allocation(the_header, size);
975+
Header_s* base = do_weak_allocation<std::is_same_v<StrongWeakLinkType, WeakLinks>>(the_header, size);
975976
container_pointer myAddress = (container_pointer)HeaderPtrToWeakPtr(base);
976977
if (!myAddress)
977978
throw_hard_error("Out of memory in allocate");
@@ -999,7 +1000,7 @@ template <class VT> class GCWeakPointerAllocator {
9991000
#ifdef DEBUG_GCWEAK
10001001
printf("%s:%d Allocating WeakPointer with GC_MALLOC_ATOMIC\n", __FILE__, __LINE__);
10011002
#endif
1002-
Header_s* base = do_weak_allocation(the_header, size);
1003+
Header_s* base = do_weak_allocation<true>(the_header, size);
10031004
VT* myAddress = (VT*)HeaderPtrToWeakPtr(base);
10041005
if (!myAddress)
10051006
throw_hard_error("Out of memory in allocate");

include/clasp/gctools/gcalloc_boehm.h

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@
44
#define ALIGNED_GC_MALLOC(sz) MAYBE_MONITOR_ALLOC(GC_MALLOC(sz), sz)
55
#define ALIGNED_GC_MALLOC_ATOMIC(sz) MAYBE_MONITOR_ALLOC(GC_MALLOC_ATOMIC(sz), sz)
66
#define ALIGNED_GC_MALLOC_UNCOLLECTABLE(sz) MAYBE_MONITOR_ALLOC(GC_MALLOC_UNCOLLECTABLE(sz), sz)
7-
#define ALIGNED_GC_MALLOC_KIND(stmp, sz, knd, kndaddr) MAYBE_MONITOR_ALLOC(GC_malloc_kind_global(sz, knd), sz)
7+
#define ALIGNED_GC_MALLOC_KIND(sz, knd) MAYBE_MONITOR_ALLOC(GC_malloc_kind_global(sz, knd), sz)
88
#define ALIGNED_GC_MALLOC_STRONG_WEAK_KIND(sz, knd) MAYBE_MONITOR_ALLOC(GC_malloc_kind_global(sz, knd), sz)
99
#define ALIGNED_GC_MALLOC_ATOMIC_KIND(stmp, sz, knd, kndaddr) \
1010
MAYBE_MONITOR_ALLOC( \
1111
(knd == GC_I_PTRFREE) ? GC_malloc_kind_global(sz, knd) : malloc_kind_error(GC_I_PTRFREE, knd, sz, stmp, kndaddr), sz)
12-
#define ALIGNED_GC_MALLOC_UNCOLLECTABLE_KIND(stmp, sz, knd, kndaddr) \
12+
#define ALIGNED_GC_MALLOC_UNCOLLECTABLE_KIND(sz, knd) \
1313
MAYBE_MONITOR_ALLOC(GC_generic_malloc_uncollectable(sz, knd), sz)
1414
#else
1515
#error "There is more work to do to support more than 3 tag bits"
@@ -24,7 +24,7 @@ template <typename Stage, typename Cons, typename... ARGS> inline Cons* do_cons_
2424
RAIIAllocationStage<Stage> stage(my_thread_low_level);
2525
#ifdef USE_PRECISE_GC
2626
ConsHeader_s* header = reinterpret_cast<ConsHeader_s*>(
27-
ALIGNED_GC_MALLOC_KIND(STAMP_UNSHIFT_WTAG(STAMPWTAG_CONS), size, global_cons_kind, &global_cons_kind)); // wasMTAG
27+
ALIGNED_GC_MALLOC_KIND(size, global_cons_kind)); // wasMTAG
2828
#ifdef DEBUG_BOEHMPRECISE_ALLOC
2929
printf("%s:%d:%s cons = %p\n", __FILE__, __LINE__, __FUNCTION__, cons);
3030
#endif
@@ -62,14 +62,14 @@ inline Header_s* do_atomic_allocation(const Header_s::StampWtagMtag& the_header,
6262
return header;
6363
};
6464

65+
template <bool weakp>
6566
inline Header_s* do_weak_allocation(const Header_s::StampWtagMtag& the_header, size_t size) {
6667
size_t true_size = size;
67-
#ifdef USE_PRECISE_GC
68-
Header_s* header = reinterpret_cast<Header_s*>(ALIGNED_GC_MALLOC_ATOMIC(true_size));
69-
// Header_s* header = reinterpret_cast<Header_s*>(ALIGNED_GC_MALLOC_STRONG_WEAK_KIND_ATOMIC(true_size,global_strong_weak_kind));
70-
#else
71-
Header_s* header = reinterpret_cast<Header_s*>(ALIGNED_GC_MALLOC_ATOMIC(true_size));
72-
#endif
68+
Header_s* header;
69+
if constexpr(weakp)
70+
header = reinterpret_cast<Header_s*>(ALIGNED_GC_MALLOC_ATOMIC(true_size));
71+
else
72+
header = reinterpret_cast<Header_s*>(ALIGNED_GC_MALLOC_KIND(true_size, GC_I_NORMAL));
7373
my_thread_low_level->_Allocations.registerWeakAllocation(the_header._value, true_size);
7474
#ifdef DEBUG_GUARD
7575
memset(header, 0x00, true_size);
@@ -92,7 +92,7 @@ inline Header_s* do_general_allocation(const Header_s::StampWtagMtag& the_header
9292
auto stamp = the_header.stamp();
9393
auto& kind = global_stamp_layout[stamp].boehm._kind;
9494
GCTOOLS_ASSERT(kind != KIND_UNDEFINED);
95-
Header_s* header = reinterpret_cast<Header_s*>(ALIGNED_GC_MALLOC_KIND(stamp, true_size, kind, &kind));
95+
Header_s* header = reinterpret_cast<Header_s*>(ALIGNED_GC_MALLOC_KIND(true_size, kind));
9696
#ifdef DEBUG_BOEHMPRECISE_ALLOC
9797
printf("%s:%d:%s header = %p\n", __FILE__, __LINE__, __FUNCTION__, header);
9898
#endif
@@ -117,8 +117,7 @@ inline Header_s* do_uncollectable_allocation(const Header_s::StampWtagMtag& the_
117117
#endif
118118
#ifdef USE_PRECISE_GC
119119
Header_s* header = reinterpret_cast<Header_s*>(
120-
ALIGNED_GC_MALLOC_UNCOLLECTABLE_KIND(the_header.stamp(), true_size, global_stamp_layout[the_header.stamp()].boehm._kind,
121-
&global_stamp_layout[the_header.stamp()].boehm._kind));
120+
ALIGNED_GC_MALLOC_UNCOLLECTABLE_KIND(true_size, global_stamp_layout[the_header.stamp()].boehm._kind));
122121
#ifdef DEBUG_BOEHMPRECISE_ALLOC
123122
printf("%s:%d:%s header = %p\n", __FILE__, __LINE__, __FUNCTION__, header);
124123
#endif

0 commit comments

Comments
 (0)