erts_factory_undo: fix main heap not reset correctly bug

When initializing a heap factory with the function
erts_factory_proc_prealloc_init, the preallocated heap space can be
placed in a heap fragment if the main heap does not have enough
space. In that case, a pointer to the first allocated word in the
fragment is saved in the factory's heap_start field so that the heap
can be reset (if an erts_factory_undo call is made later) but no
pointer to the heap top of the main heap is saved. However, space on
the main heap can be allocated later by the factory if the function
erts_produce_heap is called with the factory as argument. Before this
fix there would then be no way to restore the main heap in an
erts_factory_undo call, because the previous main heap top would be
lost.

This commit fixes the issue described above by introducing a new field
in the factory struct that stores the top of the main heap at the time
the factory is initialized. This field is used to restore the main
heap top in the erts_factory_undo function.

2 files changed, 36 insertions, 4 deletions

diff --git a/erts/emulator/beam/erl_message.c b/erts/emulator/beam/erl_message.c
index 6645341512..3c6dcf2804 100644
--- a/erts/emulator/beam/erl_message.c
+++ b/erts/emulator/beam/erl_message.c
@@ -1147,6 +1147,7 @@ void erts_factory_proc_init(ErtsHeapFactory* factory, Process* p)
     factory->mode     = FACTORY_HALLOC;
     factory->p        = p;
     factory->hp_start = HEAP_TOP(p);
+    factory->original_htop = factory->hp_start;
     factory->hp       = factory->hp_start;
     factory->hp_end   = HEAP_LIMIT(p);
     factory->off_heap = &p->off_heap;
@@ -1168,6 +1169,11 @@ void erts_factory_proc_prealloc_init(ErtsHeapFactory* factory,
     ErlHeapFragment *bp = p->mbuf;
     factory->mode     = FACTORY_HALLOC;
     factory->p        = p;
+    factory->original_htop = HEAP_TOP(p);
+    /*
+       factory->hp_start is a pointer to somewhere in the data area of
+       a heap fragment or to the main heap.
+    */
     factory->hp_start = HAlloc(p, size);
     factory->hp       = factory->hp_start;
     factory->hp_end   = factory->hp_start + size;
@@ -1234,6 +1240,12 @@ erts_factory_message_create(ErtsHeapFactory* factory,
 	ASSERT(ohp == &proc->off_heap);
 	factory->mode = FACTORY_HALLOC;
 	factory->p = proc;
+        /*
+          If on_heap is set then hp must be on the process main heap.
+         */
+        factory->original_htop = hp;
+        ASSERT(HEAP_START(proc) <= factory->original_htop);
+        ASSERT(factory->original_htop <= HEAP_LIMIT(proc));
 	factory->heap_frags_saved = proc->mbuf;
 	factory->heap_frags_saved_used = proc->mbuf ? proc->mbuf->used_size : 0;
     }
@@ -1566,10 +1578,10 @@ void erts_factory_undo(ErtsHeapFactory* factory)
             /* Rollback heap top
 	     */
 
-	    if (HEAP_START(factory->p) <= factory->hp_start
-		&& factory->hp_start <= HEAP_LIMIT(factory->p)) {
-		HEAP_TOP(factory->p) = factory->hp_start;
-	    }
+            ASSERT(HEAP_START(factory->p) <= factory->original_htop);
+            ASSERT(factory->original_htop <= HEAP_LIMIT(factory->p));
+            HEAP_TOP(factory->p) = factory->original_htop;
+
 
 	    /* Fix last heap frag */
             if (factory->heap_frags_saved) {
diff --git a/erts/emulator/beam/erl_message.h b/erts/emulator/beam/erl_message.h
index 5bd25737a7..9317d5022d 100644
--- a/erts/emulator/beam/erl_message.h
+++ b/erts/emulator/beam/erl_message.h
@@ -74,7 +74,27 @@ typedef struct {
         FACTORY_TMP
     } mode;
     Process* p;
+    /*
+       If the factory is initialized with erts_factory_proc_prealloc_init,
+       hp_start points to the top of the main heap if the preallocated data
+       fits in the main heap and otherwise it points to somewhere in the
+       data area of a heap fragment. If the factory is initialized with any
+       of the other init functions that sets the mode to FACTORY_HALLOC,
+       hp_start and original_htop always have the same value.
+
+       When erts_factory_proc_prealloc_init is used for initialization the
+       preallocated data might be allocated in an existing heap fragment but
+       data that is later allocated with erts_produce_heap might fit in the
+       main heap, so both hp_start and original_htop are needed to correctly
+       restore the heap in the erts_factory_undo function.
+    */
     Eterm* hp_start;
+    /*
+       original_htop stores the top of the main heap at the time
+       the factory was initialized and is used to reset the heap
+       state if an erts_factory_undo call is made.
+    */
+    Eterm* original_htop;
     Eterm* hp;
     Eterm* hp_end;
     ErtsMessage *message;