diff --git a/src/hx/Hash.h b/src/hx/Hash.h index 6614f6417..0fcc5d5a4 100644 --- a/src/hx/Hash.h +++ b/src/hx/Hash.h @@ -270,6 +270,7 @@ struct HashRoot : public Object HX_IS_INSTANCE_OF enum { _hx_ClassId = hx::clsIdHash }; virtual void updateAfterGc() = 0; + virtual bool markWeakValues(MarkContext *__inCtx) = 0; inline int getSize() { return size; } }; @@ -352,6 +353,46 @@ struct Hash : public HashBase< typename ELEMENT::Key > bool TIsWeakRefValid(Dynamic &key) { return IsWeakRefValid(key.mPtr); } bool TIsWeakRefValid(String &key) { return IsWeakRefValid(key.raw_ptr()); } + template + bool TMarkWeakValue(T &, MarkContext *) { return false; } + bool TMarkWeakValue(Dynamic &inValue, MarkContext *__inCtx) + { + hx::Object *value = inValue.mPtr; + if (!value || (((unsigned int *)value)[-1] & hx::gPrevMarkIdMask)) + return false; + + HX_MARK_MEMBER(inValue); + return true; + } + bool TMarkWeakValue(String &inValue, MarkContext *__inCtx) + { + const HX_CHAR *value = inValue.raw_ptr(); + if (!value || (((unsigned int *)value)[-1] & hx::gPrevMarkIdMask)) + return false; + + HX_MARK_MEMBER(inValue); + return true; + } + + bool markWeakValues(MarkContext *__inCtx) HXCPP_OVERRIDE + { + if (!Element::WeakKeys || !Element::ManageKeys) + return false; + + bool marked = false; + for(int b=0;bkey)) + marked = TMarkWeakValue(element->value, __inCtx) || marked; + element = element->next; + } + } + return marked; + } + void updateAfterGc() HXCPP_OVERRIDE { @@ -787,8 +828,8 @@ struct Hash : public HashBase< typename ELEMENT::Key > if (!Hash::Element::WeakKeys) { HX_MARK_MEMBER(inElem->key); + HX_MARK_MEMBER(inElem->value); } - HX_MARK_MEMBER(inElem->value); } }; diff --git a/src/hx/gc/Immix.cpp b/src/hx/gc/Immix.cpp index b59a6ef48..8d69ad156 100644 --- a/src/hx/gc/Immix.cpp +++ b/src/hx/gc/Immix.cpp @@ -4772,6 +4772,32 @@ class GlobalAllocator hx::FindZombies(mMarker); + // Weak-key hashes are ephemerons: a value is reachable only when both + // the hash and its key are reachable. Values marked here may in turn + // make keys in other weak hashes reachable, so iterate to a fixed point. + bool markedWeakValue; + do + { + markedWeakValue = false; + mMarker.init(); + for(int i=0;imarkWeakValues(&mMarker) || markedWeakValue; + } + if (markedWeakValue) + { + #ifdef HX_MULTI_THREAD_MARKING + mMarker.releaseJobs(); + StartThreadJobs(tpjMark, MAX_GC_THREADS, true); + #else + mMarker.processMarkStack(); + #endif + } + } + while(markedWeakValue); + hx::RunFinalizers(); #ifdef HXCPP_GC_VERIFY diff --git a/test/haxe/TestWeakHash.hx b/test/haxe/TestWeakHash.hx index 385570be5..962a04bfb 100644 --- a/test/haxe/TestWeakHash.hx +++ b/test/haxe/TestWeakHash.hx @@ -9,6 +9,12 @@ class WeakObjectData public function toString() return "Data " + id; } +class WeakValueData +{ + public var key:WeakObjectData; + public function new(inKey:WeakObjectData) key = inKey; +} + class TestWeakHash extends Test { var retained:Array; @@ -104,4 +110,95 @@ class TestWeakHash extends Test Assert.pass(); } + public function testDeadKeyReleasesValueCycle() + { + var result:{ + map:WeakMap, + key:cpp.vm.WeakRef, + value:cpp.vm.WeakRef + } = null; + + final sema = new sys.thread.Semaphore(0); + sys.thread.Thread.create(() -> { + var map = new WeakMap(); + var key = new WeakObjectData(1); + var value = new WeakValueData(key); + map.set(key,value); + result = { + map: map, + key: new cpp.vm.WeakRef(key), + value: new cpp.vm.WeakRef(value) + }; + sema.release(); + }); + sema.acquire(); + + // Ensure no conservative reference remains on the terminated thread stack. + Sys.sleep(1); + cpp.vm.Gc.run(true); + + Assert.isNull(result.key.get()); + Assert.isNull(result.value.get()); + Assert.isFalse(result.map.keys().hasNext()); + } + + public function testLiveKeyRetainsValue() + { + var map:WeakMap = null; + var value:cpp.vm.WeakRef = null; + + final sema = new sys.thread.Semaphore(0); + sys.thread.Thread.create(() -> { + map = new WeakMap(); + var key = new WeakObjectData(2); + var mapValue = new WeakValueData(key); + retained = [key]; + map.set(key,mapValue); + value = new cpp.vm.WeakRef(mapValue); + sema.release(); + }); + sema.acquire(); + + Sys.sleep(1); + cpp.vm.Gc.run(true); + + Assert.notNull(value.get()); + Assert.notNull(map.get(retained[0])); + } + + public function testEphemeronsReachFixedPoint() + { + var maps:{ + upstream:WeakMap, + downstream:WeakMap + } = null; + var value:cpp.vm.WeakRef = null; + + final sema = new sys.thread.Semaphore(0); + sys.thread.Thread.create(() -> { + // Register downstream first so it is visited before upstream. A + // single pass would skip it before upstream makes its key reachable. + var downstream = new WeakMap(); + var upstream = new WeakMap(); + var rootKey = new WeakObjectData(3); + var linkKey = new WeakObjectData(4); + var finalValue = new WeakObjectData(5); + downstream.set(linkKey,finalValue); + upstream.set(rootKey,linkKey); + retained = [rootKey]; + maps = {upstream: upstream, downstream: downstream}; + value = new cpp.vm.WeakRef(finalValue); + sema.release(); + }); + sema.acquire(); + + Sys.sleep(1); + cpp.vm.Gc.run(true); + + var linkKey = maps.upstream.get(retained[0]); + Assert.notNull(linkKey); + Assert.notNull(maps.downstream.get(linkKey)); + Assert.notNull(value.get()); + } + }