File 1332-erts-Fix-reference-counting-of-magic-refs-... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 1332-erts-Fix-reference-counting-of-magic-refs-in-ETS-com.patch of Package erlang (Revision 1e94e72e3f4fed1ca9cd5c2fd777ed02)

Currently displaying revision 1e94e72e3f4fed1ca9cd5c2fd777ed02 , Show latest

From 41febafc004070e0059129bf692deacd69117259 Mon Sep 17 00:00:00 2001
From: Sverker Eriksson <sverker@erlang.org>
Date: Thu, 29 Jun 2023 19:46:52 +0200
Subject: [PATCH] erts: Fix reference counting of magic refs in ETS compressed
 format

Store the EtrsMRefThing as is like we do for ProcBin,
in order to link them into off-heap list.
---
 erts/emulator/beam/erl_bif_info.c    |  2 +-
 erts/emulator/beam/erl_db_util.c     |  4 +--
 erts/emulator/beam/erl_db_util.h     | 26 +++++++++++------
 erts/emulator/beam/erl_node_tables.c |  2 +-
 erts/emulator/beam/external.c        | 43 ++++++++++++++++++++++++++--
 erts/emulator/beam/external.h        |  1 +
 lib/stdlib/test/ets_SUITE.erl        | 24 ++++++++++++++++
 7 files changed, 87 insertions(+), 15 deletions(-)

diff --git a/erts/emulator/beam/erl_bif_info.c b/erts/emulator/beam/erl_bif_info.c
index d5e598515c..98b0f480e0 100644
--- a/erts/emulator/beam/erl_bif_info.c
+++ b/erts/emulator/beam/erl_bif_info.c
@@ -170,7 +170,7 @@ erts_bld_bin_list(Uint **hpp, Uint *szp, ErlOffHeap* oh, Eterm tail)
     union erl_off_heap_ptr u;
     Eterm res = tail;
     Eterm tuple;
-    struct erts_tmp_aligned_offheap tmp;
+    union erts_tmp_aligned_offheap tmp;
 
     for (u.hdr = oh->first; u.hdr; u.hdr = u.hdr->next) {
         erts_align_offheap(&u, &tmp);
diff --git a/erts/emulator/beam/erl_db_util.c b/erts/emulator/beam/erl_db_util.c
index 8bace68d0f..60bad7b652 100644
--- a/erts/emulator/beam/erl_db_util.c
+++ b/erts/emulator/beam/erl_db_util.c
@@ -3389,12 +3389,12 @@ Eterm db_copy_element_from_ets(DbTableCommon* tb, Process* p,
 
 
 /* Our own "cleanup_offheap"
- * as refc-binaries may be unaligned in compressed terms
+ * as ProcBin and ErtsMRefThing may be unaligned in compressed terms
 */
 void db_cleanup_offheap_comp(DbTerm* obj)
 {
     union erl_off_heap_ptr u;
-    struct erts_tmp_aligned_offheap tmp;
+    union erts_tmp_aligned_offheap tmp;
 
     for (u.hdr = obj->first_oh; u.hdr; u.hdr = u.hdr->next) {
         erts_align_offheap(&u, &tmp);
diff --git a/erts/emulator/beam/erl_db_util.h b/erts/emulator/beam/erl_db_util.h
index 6ed1e15104..db1113676e 100644
--- a/erts/emulator/beam/erl_db_util.h
+++ b/erts/emulator/beam/erl_db_util.h
@@ -570,14 +570,15 @@ ERTS_GLB_INLINE Binary *erts_db_get_match_prog_binary_unchecked(Eterm term);
 
 /** @brief Ensure off-heap header is word aligned, make a temporary copy if
  * not. Needed when inspecting ETS off-heap lists that may contain unaligned
- * ProcBins if table is 'compressed'.
+ * ProcBin and ErtsMRefThing if table is 'compressed'.
  */
-struct erts_tmp_aligned_offheap
+union erts_tmp_aligned_offheap
 {
     ProcBin proc_bin;
+    ErtsMRefThing mref_thing;
 };
 ERTS_GLB_INLINE void erts_align_offheap(union erl_off_heap_ptr*,
-                                        struct erts_tmp_aligned_offheap* tmp);
+                                        union erts_tmp_aligned_offheap* tmp);
 
 #if ERTS_GLB_INLINE_INCL_FUNC_DEF
 
@@ -613,20 +614,27 @@ erts_db_get_match_prog_binary(Eterm term)
 
 ERTS_GLB_INLINE void
 erts_align_offheap(union erl_off_heap_ptr* ohp,
-                   struct erts_tmp_aligned_offheap* tmp)
+                   union erts_tmp_aligned_offheap* tmp)
 {
     if ((UWord)ohp->voidp % sizeof(UWord) != 0) {
         /*
-         * ETS store word unaligned ProcBins in its compressed format.
-         * Make a temporary aligned copy.
+         * ETS store word unaligned ProcBin and ErtsMRefThing in its compressed
+         * format. Make a temporary aligned copy.
          *
          * Warning, must pass (void*)-variable to memcpy. Otherwise it will
          * cause Bus error on Sparc due to false compile time assumptions
          * about word aligned memory (type cast is not enough).
          */
-        sys_memcpy(tmp, ohp->voidp, sizeof(*tmp));
-        ASSERT(tmp->proc_bin.thing_word == HEADER_PROC_BIN);
-        ohp->pb = &tmp->proc_bin;
+        sys_memcpy(tmp, ohp->voidp, sizeof(Eterm)); /* thing_word */
+        if (tmp->proc_bin.thing_word == HEADER_PROC_BIN) {
+            sys_memcpy(tmp, ohp->voidp, sizeof(tmp->proc_bin));
+            ohp->pb = &tmp->proc_bin;
+        }
+        else {
+            sys_memcpy(tmp, ohp->voidp, sizeof(tmp->mref_thing));
+            ASSERT(is_magic_ref_thing(&tmp->mref_thing));
+            ohp->mref = &tmp->mref_thing;
+        }
     }
 }
 
diff --git a/erts/emulator/beam/erl_node_tables.c b/erts/emulator/beam/erl_node_tables.c
index 912caf5ace..7b5ff65756 100644
--- a/erts/emulator/beam/erl_node_tables.c
+++ b/erts/emulator/beam/erl_node_tables.c
@@ -1514,7 +1514,7 @@ static void
 insert_offheap(ErlOffHeap *oh, int type, Eterm id)
 {
     union erl_off_heap_ptr u;
-    struct erts_tmp_aligned_offheap tmp;
+    union erts_tmp_aligned_offheap tmp;
     struct insert_offheap2_arg a;
     a.type = BIN_REF;
 
diff --git a/erts/emulator/beam/external.c b/erts/emulator/beam/external.c
index 67584e38ec..ef09681d39 100644
--- a/erts/emulator/beam/external.c
+++ b/erts/emulator/beam/external.c
@@ -3380,7 +3380,22 @@ enc_term_int(TTBEncodeContext* ctx, ErtsAtomCacheMap *acmp, Eterm obj, byte* ep,
 	    ep = enc_pid(acmp, obj, ep, dflags);
 	    break;
 
-	case REF_DEF:
+        case REF_DEF:
+            if ((dflags & DFLAG_ETS_COMPRESSED) && is_internal_magic_ref(obj)) {
+                ErtsMRefThing tmp;
+                ErtsMRefThing *mrtp = (ErtsMRefThing *) internal_ref_val(obj);
+
+                erts_refc_inc(&mrtp->mb->intern.refc, 2);
+
+                *ep++ = MAGIC_REF_INTERNAL_REF;
+                sys_memcpy(&tmp, mrtp, sizeof(ErtsMRefThing));
+                tmp.next = *off_heap;
+                sys_memcpy(ep, &tmp, sizeof(ErtsMRefThing));
+                *off_heap = (struct erl_off_heap_header*) ep;
+                ep += sizeof(ErtsMRefThing);
+                break;
+            }
+            /*else fall through */
 	case EXTERNAL_REF_DEF: {
 	    Uint32 *ref_num;
 	    Eterm sysname = (((dflags & DFLAG_ETS_COMPRESSED) && is_internal_ref(obj))
@@ -5011,6 +5026,19 @@ dec_term_atom_common:
 		*objp = make_binary(sub);
 		break;
 	    }
+        case MAGIC_REF_INTERNAL_REF:
+            {
+                ErtsMRefThing* mrtp = (ErtsMRefThing*) hp;
+                sys_memcpy(mrtp, ep, sizeof(ErtsMRefThing));
+                ep += sizeof(ErtsMRefThing);
+                erts_refc_inc(&mrtp->mb->intern.refc, 2);
+                hp += ERTS_MAGIC_REF_THING_SIZE;
+                mrtp->next = factory->off_heap->first;
+                factory->off_heap->first = (struct erl_off_heap_header*)mrtp;
+                *objp = make_internal_ref(mrtp);
+                ASSERT(is_internal_magic_ref(*objp));
+                break;
+            }
 
 	default:
 	    goto error;
@@ -5231,8 +5259,13 @@ encode_size_struct_int(TTBSizeContext* ctx, ErtsAtomCacheMap *acmp, Eterm obj,
 	    result += (1 + encode_size_struct2(acmp, pid_node_name(obj), dflags) +
 		       4 + 4 + 4);
 	    break;
+        case REF_DEF:
+            if ((dflags & DFLAG_ETS_COMPRESSED) && is_internal_magic_ref(obj)) {
+                result += 1 + sizeof(ErtsMRefThing);
+                break;
+            }
+            /* else fall through */
         case EXTERNAL_REF_DEF:
-	case REF_DEF:
 	    ASSERT(dflags & DFLAG_EXTENDED_REFERENCES);
 	    i = ref_no_numbers(obj);
 	    result += (1 + 2 + encode_size_struct2(acmp, ref_node_name(obj), dflags) +
@@ -5883,6 +5916,12 @@ init_done:
 	    SKIP(2+sizeof(ProcBin));
 	    heap_size += PROC_BIN_SIZE + ERL_SUB_BIN_SIZE;
 	    break;
+        case MAGIC_REF_INTERNAL_REF:
+            if (!internal_tags)
+                goto error;
+            SKIP(sizeof(ErtsMRefThing));
+            heap_size += ERTS_MAGIC_REF_THING_SIZE;
+            break;
 	default:
 	    goto error;
 	}
diff --git a/erts/emulator/beam/external.h b/erts/emulator/beam/external.h
index 7a59d02102..41171a3336 100644
--- a/erts/emulator/beam/external.h
+++ b/erts/emulator/beam/external.h
@@ -67,6 +67,7 @@
 #define ATOM_INTERNAL_REF3 'K'
 #define BINARY_INTERNAL_REF 'J'
 #define BIT_BINARY_INTERNAL_REF 'L'
+#define MAGIC_REF_INTERNAL_REF 'N'
 #define COMPRESSED        'P'
 
 #if 0
diff --git a/lib/stdlib/test/ets_SUITE.erl b/lib/stdlib/test/ets_SUITE.erl
index 588096301d..c006f4ed25 100644
--- a/lib/stdlib/test/ets_SUITE.erl
+++ b/lib/stdlib/test/ets_SUITE.erl
@@ -96,6 +96,7 @@
 -export([otp_9932/1]).
 -export([otp_9423/1]).
 -export([otp_10182/1]).
+-export([compress_magic_ref/1]).
 -export([ets_all/1]).
 -export([massive_ets_all/1]).
 -export([take/1]).
@@ -168,6 +169,7 @@ all() ->
      otp_10182,
      otp_9932,
      otp_9423,
+     compress_magic_ref,
      ets_all,
      massive_ets_all,
      take,
@@ -7280,6 +7282,28 @@ otp_10182(Config) when is_list(Config) ->
               In = Out
       end).
 
+%% Verify magic refs in compressed table are reference counted correctly
+compress_magic_ref(Config) when is_list(Config)->
+    F = fun(Opts) ->
+                T = ets:new(banana, Opts),
+                ets:insert(T, {key, atomics:new(2, [])}),
+                erlang:garbage_collect(),  % make really sure no ref on heap
+                [{_, Ref}] = ets:lookup(T, key),
+                #{size := 2} = atomics:info(Ref), % Still alive!
+
+                %% Now test ets:delete will deallocate if last ref
+                WeakRef = term_to_binary(Ref),
+                erlang:garbage_collect(),  % make sure no Ref on heap
+                ets:delete(T, key),
+                StaleRef = binary_to_term(WeakRef),
+                badarg = try atomics:info(StaleRef)
+                         catch error:badarg -> badarg end,
+                ets:delete(T),
+                ok
+          end,
+    repeat_for_opts(F, [[set, ordered_set], compressed]),
+    ok.
+
 %% Test that ets:all include/exclude tables that we know are created/deleted
 ets_all(Config) when is_list(Config) ->
     Pids = [spawn_link(fun() -> ets_all_run() end) || _ <- [1,2]],
-- 
2.35.3

Places

File 1332-erts-Fix-reference-counting-of-magic-refs-in-ETS-com.patch of Package erlang (Revision 1e94e72e3f4fed1ca9cd5c2fd777ed02)

Places