Browse Source

Hopefully fixed the allocator failure to account memory properly

Caleb James DeLisle 6 years ago
parent
commit
41efd0c895
3 changed files with 41 additions and 6 deletions
  1. 1 0
      .gitignore
  2. 38 4
      memory/Allocator.c
  3. 2 2
      memory/Allocator_pvt.h

+ 1 - 0
.gitignore

@@ -31,3 +31,4 @@
 /node_modules/jshint
 /node_modules/.bin
 /contrib/docker/cjdns*/
+.vscode

+ 38 - 4
memory/Allocator.c

@@ -78,6 +78,14 @@ static inline uint64_t bytesAllocated(struct Allocator_pvt* ctx)
     return bytes;
 }
 
+static void check(struct Allocator_pvt* alloc)
+{
+    if (!Defined(Allocator_PARANOIA)) { return; }
+    uint64_t totalAllocated = alloc->rootAlloc->maxSpace - alloc->rootAlloc->spaceAvailable;
+    uint64_t accounted = bytesAllocated(Identity_check((struct Allocator_pvt*)alloc->rootAlloc));
+    Assert_true(totalAllocated == accounted);
+}
+
 void Allocator_snapshot(struct Allocator* allocator, int includeAllocations)
 {
     // get the root allocator.
@@ -156,6 +164,7 @@ static inline void* newAllocation(struct Allocator_pvt* context,
                                   const char* fileName,
                                   int lineNum)
 {
+    check(context);
     int64_t realSize = getRealSize(size);
     struct Allocator_FirstCtx* rootAlloc = Identity_check(context->rootAlloc);
     if (rootAlloc->spaceAvailable <= realSize) {
@@ -353,6 +362,7 @@ static int pivotChildrenToAdoptedParents0(struct Allocator_pvt* context,
 {
     int out = 0;
     if (depth == maxDepth) {
+        if (context->pub.isFreeing) { return 0; }
         if (context->adoptions) {
             // Attempt to pivot around to a parent in order to save this allocator
             if (context->adoptions->parents) {
@@ -368,8 +378,6 @@ static int pivotChildrenToAdoptedParents0(struct Allocator_pvt* context,
                 disconnectAdopted(context, c->alloc);
             }
         }
-        // If (context->parent == context) it's a root allocator and disconnect does the wrong thing
-        if (depth == 0 && context->parent != context) { disconnect(context); }
         Assert_true(!context->pub.isFreeing);
         context->pub.isFreeing = 1;
         out++;
@@ -403,6 +411,7 @@ static int pivotChildrenToAdoptedParents(struct Allocator_pvt* context, const ch
  */
 static void marshalOnFreeJobs(struct Allocator_pvt* context, struct Allocator_pvt* rootToFree)
 {
+    Assert_true(context->pub.isFreeing);
     struct Allocator_pvt* child = context->firstChild;
     while (child) {
         // Theoretically the order of free jobs is not promised but this prevents libuv crashing.
@@ -412,6 +421,7 @@ static void marshalOnFreeJobs(struct Allocator_pvt* context, struct Allocator_pv
             jobP = &job->next;
         }
         *jobP = child->onFree;
+        child->onFree = NULL;
 
         while (*jobP != NULL) {
             struct Allocator_OnFreeJob_pvt* job = *jobP;
@@ -435,10 +445,12 @@ static void doOnFreeJobs(struct Allocator_pvt* context)
             // no callback, remove the job
             Assert_true(!removeJob(job));
             continue;
-        } else {
+        } else if (!job->called) {
             if  (job->pub.callback(&job->pub) != Allocator_ONFREE_ASYNC) {
                 Assert_true(!removeJob(job));
                 continue;
+            } else {
+                job->called = 1;
             }
         }
         jobP = &job->next;
@@ -447,6 +459,12 @@ static void doOnFreeJobs(struct Allocator_pvt* context)
 
 static void freeAllocator(struct Allocator_pvt* context)
 {
+    Assert_true(context->pub.isFreeing);
+    int isTop = !context->parent->pub.isFreeing;
+    if (isTop) {
+        check(context);
+        disconnect(context);
+    }
     struct Allocator_pvt* child = context->firstChild;
     while (child) {
         struct Allocator_pvt* nextChild = child->nextSibling;
@@ -460,6 +478,9 @@ static void freeAllocator(struct Allocator_pvt* context)
     Allocator_Provider_CONTEXT_TYPE* providerCtx = rootAlloc->providerContext;
 
     releaseMemory(context, provider, providerCtx);
+    if (isTop) {
+        check((struct Allocator_pvt*)rootAlloc);
+    }
 }
 
 void Allocator_onFreeComplete(struct Allocator_OnFreeJob* onFreeJob)
@@ -480,6 +501,7 @@ void Allocator_onFreeComplete(struct Allocator_OnFreeJob* onFreeJob)
 void Allocator__free(struct Allocator* alloc, const char* file, int line)
 {
     struct Allocator_pvt* context = Identity_check((struct Allocator_pvt*) alloc);
+    check(context);
 
     // It's really difficult to know that you didn't get called back inside of a freeing of a
     // parent of a parent allocator which causes your allocator to be in isFreeing state so
@@ -493,9 +515,13 @@ void Allocator__free(struct Allocator* alloc, const char* file, int line)
         }
     }
 
+    check(context);
     if (!pivotChildrenToAdoptedParents(context, file, line)) { return; }
+    check(context);
     marshalOnFreeJobs(context, context);
+    check(context);
     doOnFreeJobs(context);
+    check(context);
     if (!context->onFree) {
         freeAllocator(context);
     }
@@ -507,7 +533,9 @@ void* Allocator__malloc(struct Allocator* allocator,
                         int lineNum)
 {
     struct Allocator_pvt* ctx = Identity_check((struct Allocator_pvt*) allocator);
-    return newAllocation(ctx, length, fileName, lineNum);
+    void* out = newAllocation(ctx, length, fileName, lineNum);
+    check(ctx);
+    return out;
 }
 
 void* Allocator__calloc(struct Allocator* alloc,
@@ -532,6 +560,7 @@ void* Allocator__realloc(struct Allocator* allocator,
     }
 
     struct Allocator_pvt* context = Identity_check((struct Allocator_pvt*) allocator);
+    check(context);
     struct Allocator_Allocation_pvt** locPtr = &context->allocations;
     struct Allocator_Allocation_pvt* origLoc =
         ((struct Allocator_Allocation_pvt*) original) - 1;
@@ -562,6 +591,7 @@ void* Allocator__realloc(struct Allocator* allocator,
                           origLoc,
                           context->rootAlloc->provider,
                           context->rootAlloc->providerContext);
+        check(context);
         return NULL;
     }
 
@@ -588,6 +618,7 @@ void* Allocator__realloc(struct Allocator* allocator,
     *locPtr = alloc;
 
     setCanaries(alloc, context);
+    check(context);
 
     return (void*) (alloc + 1);
 }
@@ -606,6 +637,7 @@ void* Allocator__clone(struct Allocator* allocator,
 struct Allocator* Allocator__child(struct Allocator* allocator, const char* file, int line)
 {
     struct Allocator_pvt* parent = Identity_check((struct Allocator_pvt*) allocator);
+    check(parent);
 
     struct Allocator_pvt stackChild = {
         .pub = {
@@ -626,6 +658,7 @@ struct Allocator* Allocator__child(struct Allocator* allocator, const char* file
     // Link the child into the parent's allocator list
     connect(parent, child, file, line);
 
+    check(parent);
     return &child->pub;
 }
 
@@ -808,6 +841,7 @@ struct Allocator* Allocator_new(unsigned long sizeLimit,
     context->rootAlloc = firstContext;
     context->parent = context;
 
+    check(context);
     return &context->pub;
 }
 

+ 2 - 2
memory/Allocator_pvt.h

@@ -28,8 +28,8 @@ struct Allocator_OnFreeJob_pvt {
     struct Allocator_pvt* alloc;
     struct Allocator_OnFreeJob_pvt* next;
 
-    /* prevent async jobs from being called multiple times, nonzero = done */
-    int done;
+    /* prevent async jobs from being called multiple times, nonzero = called */
+    int called;
 
     int line;
     const char* file;