Fix possible double free during context destruction

src/refcount.c

@@ -95,23 +95,32 @@ refcount_make_context(size_t entry_offset,
 }
 
 /**
- * Cleanup a #RefcountContext and free any associated resources. This also
+ * Callback that deinits a static, but sets its static_entry field to NULL
- * causes all remaining references, both static and non-static, to be dropped
+ * first. Used in #refcount_context_destroy.
- * and the context's destroy callback to be called on each object.
+ */
+static void deinit_static_for_context_destroy(void *obj, void *ctx_raw) {
+    RefcountContext *ctx = ctx_raw;
+    ENTRY->impl.static_entry = NULL;
+    refcount_context_deinit_static(ctx, obj);
+}
+
+/**
+ * Cleanup a #RefcountContext and free any associated resources. This first
+ * frees all static objects, then runs the garbage collector.
  * @param ctx The #RefcountContext
  */
 void refcount_context_destroy(RefcountContext *ctx) {
-    refcount_list_free_with_data_full(
+    refcount_list_free_with_data_full(ctx->static_objects,
-        ctx->static_objects, refcount_context_deinit_static_as_callback, ctx,
+                                      deinit_static_for_context_destroy, ctx,
-        &ctx->alloc);
+                                      &ctx->alloc);
+
+    refcount_context_garbage_collect(ctx);
 
 #ifdef REFCOUNT_HAS_THREADS
     mtx_destroy(&ctx->so_mtx);
     mtx_destroy(&ctx->gr_mtx);
 #endif
 
-    refcount_context_garbage_collect(ctx);
-
     refcount_free(&ctx->alloc, ctx);
 }
 
@@ -325,8 +334,11 @@ bool refcount_context_deinit_static(RefcountContext *ctx, void *obj) {
     ht_free(ENTRY->destructors);
     unref_weakref(ctx, ENTRY->weak_ref);
     unlock_entry_mtx(ENTRY);
-    ctx->static_objects = refcount_list_remove_full(
+    // this is set to null if we are destroying the context
-        ctx->static_objects, ENTRY->impl.static_entry, NULL, &ctx->alloc);
+    if (ENTRY->impl.static_entry) {
+        ctx->static_objects = refcount_list_remove_full(
+            ctx->static_objects, ENTRY->impl.static_entry, NULL, &ctx->alloc);
+    }
     refcount_list_free_with_data_full(
         held_refs, refcount_context_unref_as_callback, ctx, &ctx->alloc);
     success = true;

test/test_refcount.c

@@ -55,12 +55,16 @@ int main(int argc, const char **argv) {
 
     ctx_and_flag.ctx = c;
 
-    A static_a = {
+#define STATIC_A(id)                            \
-        .num = 0,
+    A static_a_##id = {                         \
-        .str = counting_strdup("static"),
+        .num = __LINE__,                        \
-        .next = make_a(c, 0, "in static"),
+        .str = counting_strdup("static " #id),  \
-    };
+        .next = make_a(c, 0, "in static " #id), \
-    refcount_context_init_static(c, &static_a);
+    };                                          \
+    refcount_context_init_static(c, &static_a_##id)
+    STATIC_A(1);
+    STATIC_A(2);
+    STATIC_A(3);
 
     A *a = make_a(c, 10, "Hello world\n");
     assert(!refcount_context_is_static(c, a));
@@ -187,10 +191,12 @@ int main(int argc, const char **argv) {
     assert(refcount_context_num_refs(c, a) == 1);
     assert(!refcount_context_unref(c, a));
 
-    refcount_context_deinit_static(c, &static_a);
+    refcount_context_deinit_static(c, &static_a_1);
-    counting_free(static_a.str);
+    counting_free(static_a_1.str);
 
     refcount_context_destroy(c);
+    counting_free(static_a_2.str);
+    counting_free(static_a_3.str);
 
     check_allocator_status();
     return 0;