nix_api_expr: switch to refcounting

Remove GCRef, keep references in a map. Change to nix_gc_incref and
nix_gc_decref, where users will mostly use nix_gc_decref.

src/libexpr/nix_api_expr.cc

@@ -16,6 +16,7 @@
 #include "nix_api_util_internal.h"
 
 #ifdef HAVE_BOEHMGC
+#include <mutex>
 #define GC_INCLUDE_NEW 1
 #include "gc_cpp.h"
 #endif
@@ -100,27 +101,42 @@ State *nix_state_create(nix_c_context *context, const char **searchPath_c,
 
 void nix_state_free(State *state) { delete state; }
 
-GCRef *nix_gc_ref(nix_c_context *context, void *obj) {
+#ifdef HAVE_BOEHMGC
-  if (context)
+std::unordered_map<
-    context->last_err_code = NIX_OK;
+    const void *, unsigned int, std::hash<const void *>,
-  try {
+    std::equal_to<const void *>,
-#if HAVE_BOEHMGC
+    traceable_allocator<std::pair<const void *const, unsigned int>>>
-    return new (NoGC) GCRef{obj};
+    nix_refcounts;
-#else
+
-    return new GCRef{obj};
+std::mutex nix_refcount_lock;
-#endif
+
+void nix_gc_incref(const void *p) {
+  std::scoped_lock lock(nix_refcount_lock);
+  auto f = nix_refcounts.find(p);
+  if (f != nix_refcounts.end()) {
+    f->second++;
+  } else {
+    nix_refcounts[p] = 1;
   }
-  NIXC_CATCH_ERRS_NULL
 }
 
-void nix_gc_free(GCRef *ref) {
+void nix_gc_decref(const void *p) {
-#if HAVE_BOEHMGC
+  std::scoped_lock lock(nix_refcount_lock);
-  GC_FREE(ref);
+  auto f = nix_refcounts.find(p);
-#else
+  if (f != nix_refcounts.end()) {
-  delete ref;
+    if (f->second == 1)
-#endif
+      nix_refcounts.erase(f);
+    else
+      f->second--;
+  }
+  // todo: else { throw? }
 }
 
+#else
+void nix_gc_incref(const void *){};
+void nix_gc_decref(const void *){};
+#endif
+
 void nix_gc_register_finalizer(void *obj, void *cd,
                                void (*finalizer)(void *obj, void *cd)) {
 #ifdef HAVE_BOEHMGC

src/libexpr/nix_api_expr.h

@@ -26,14 +26,6 @@ typedef struct State State; // nix::EvalState
  * Owned by the GC.
  */
 typedef void Value; // nix::Value
-/**
- * @brief Reference for the GC
- *
- * Nix uses a garbage collector that may not be able to see into
- * your stack and heap. Keep GCRef objects around for every
- * garbage-collected object that you want to keep alive.
- */
-typedef struct GCRef GCRef; // void*
 
 // Function prototypes
 /**
@@ -119,22 +111,22 @@ State *nix_state_create(nix_c_context *context, const char **searchPath,
 void nix_state_free(State *state);
 
 /**
- * @brief Creates a new garbage collector reference.
+ * @brief Increase the GC refcount.
  *
- * @param[out] context Optional, stores error information
+ * The nix C api keeps alive objects by refcounting.
- * @param[in] obj The object to create a reference for.
+ * When you're done with a refcounted pointer, call nix_gc_decref.
- * @return A new garbage collector reference or NULL on failure.
+ *
+ * Does not fail
+ *
+ * @param[in] object The object to keep alive
  */
-GCRef *nix_gc_ref(nix_c_context *context, void *obj);
+void nix_gc_incref(const void *);
-
 /**
- * @brief Frees a garbage collector reference.
+ * @brief Decrease the GC refcount
  *
- * Does not fail.
+ * @param[in] object The object to stop referencing
- *
- * @param[in] ref The reference to free.
  */
-void nix_gc_free(GCRef *ref);
+void nix_gc_decref(const void *);
 
 /**
  * @brief Register a callback that gets called when the object is garbage

src/libexpr/nix_api_expr_internal.h

@@ -11,10 +11,6 @@ struct State {
   nix::EvalState state;
 };
 
-struct GCRef {
-  void *ptr;
-};
-
 struct BindingsBuilder {
   nix::BindingsBuilder builder;
 };

src/libexpr/nix_api_external.cc

@@ -169,8 +169,7 @@ public:
 };
 
 ExternalValue *nix_create_external_value(nix_c_context *context,
-                                         NixCExternalValueDesc *desc, void *v,
+                                         NixCExternalValueDesc *desc, void *v) {
-                                         GCRef *gc) {
   if (context)
     context->last_err_code = NIX_OK;
   try {
@@ -179,8 +178,7 @@ ExternalValue *nix_create_external_value(nix_c_context *context,
         (GC)
 #endif
             NixCExternalValue(*desc, v);
-    if (gc)
+    nix_gc_incref(ret);
-      gc->ptr = ret;
     return (ExternalValue *)ret;
   }
   NIXC_CATCH_ERRS_NULL

src/libexpr/nix_api_external.h

@@ -159,18 +159,17 @@ typedef struct NixCExternalValueDesc {
 /**
  * @brief Create an external value, that can be given to nix_set_external
  *
- * Pass a gcref to keep a reference.
+ * Owned by the GC. Use nix_gc_decref when you're done with the pointer.
+ *
  * @param[out] context Optional, stores error information
  * @param[in] desc a NixCExternalValueDesc, you should keep this alive as long
  * as the ExternalValue lives
  * @param[in] v the value to store
- * @param[out] ref Optional, will store a reference to the returned value.
  * @returns external value, owned by the garbage collector
  * @see nix_set_external
  */
 ExternalValue *nix_create_external_value(nix_c_context *context,
-                                         NixCExternalValueDesc *desc, void *v,
+                                         NixCExternalValueDesc *desc, void *v);
-                                         GCRef *ref);
 
 /**
  * @brief Extract the pointer from a nix c external value.

src/libexpr/nix_api_value.cc

@@ -32,8 +32,7 @@ static nix::Value &check_value_not_null(Value *value) {
 }
 
 PrimOp *nix_alloc_primop(nix_c_context *context, PrimOpFun fun, int arity,
-                         const char *name, const char **args, const char *doc,
+                         const char *name, const char **args, const char *doc) {
-                         GCRef *ref) {
   if (context)
     context->last_err_code = NIX_OK;
   try {
@@ -50,20 +49,18 @@ PrimOp *nix_alloc_primop(nix_c_context *context, PrimOpFun fun, int arity,
     if (args)
       for (size_t i = 0; args[i]; i++)
         p->args.emplace_back(*args);
-    if (ref)
+    nix_gc_incref(p);
-      ref->ptr = p;
     return (PrimOp *)p;
   }
   NIXC_CATCH_ERRS_NULL
 }
 
-Value *nix_alloc_value(nix_c_context *context, State *state, GCRef *ref) {
+Value *nix_alloc_value(nix_c_context *context, State *state) {
   if (context)
     context->last_err_code = NIX_OK;
   try {
     Value *res = state->state.allocValue();
-    if (ref)
+    nix_gc_incref(res);
-      ref->ptr = res;
     return res;
   }
   NIXC_CATCH_ERRS_NULL
@@ -204,19 +201,21 @@ ExternalValue *nix_get_external(nix_c_context *context, Value *value) {
 }
 
 Value *nix_get_list_byidx(nix_c_context *context, const Value *value,
-                          unsigned int ix, GCRef *ref) {
+                          unsigned int ix) {
   if (context)
     context->last_err_code = NIX_OK;
   try {
     auto &v = check_value_not_null(value);
     assert(v.type() == nix::nList);
-    return (Value *)v.listElems()[ix];
+    auto *p = v.listElems()[ix];
+    nix_gc_incref(p);
+    return (Value *)p;
   }
   NIXC_CATCH_ERRS_NULL
 }
 
 Value *nix_get_attr_byname(nix_c_context *context, const Value *value,
-                           State *state, const char *name, GCRef *ref) {
+                           State *state, const char *name) {
   if (context)
     context->last_err_code = NIX_OK;
   try {
@@ -225,8 +224,7 @@ Value *nix_get_attr_byname(nix_c_context *context, const Value *value,
     nix::Symbol s = state->state.symbols.create(name);
     auto attr = v.attrs->get(s);
     if (attr) {
-      if (ref)
+      nix_gc_incref(attr->value);
-        ref->ptr = attr->value;
       return attr->value;
     }
     nix_set_err_msg(context, NIX_ERR_KEY, "missing attribute");
@@ -252,16 +250,14 @@ bool nix_has_attr_byname(nix_c_context *context, const Value *value,
 }
 
 Value *nix_get_attr_byidx(nix_c_context *context, const Value *value,
-                          State *state, unsigned int i, const char **name,
+                          State *state, unsigned int i, const char **name) {
-                          GCRef *ref) {
   if (context)
     context->last_err_code = NIX_OK;
   try {
     auto &v = check_value_not_null(value);
     const nix::Attr &a = (*v.attrs)[i];
     *name = ((const std::string &)(state->state.symbols[a.name])).c_str();
-    if (ref)
+    nix_gc_incref(a.value);
-      ref->ptr = a.value;
     return a.value;
   }
   NIXC_CATCH_ERRS_NULL

src/libexpr/nix_api_value.h

@@ -33,7 +33,6 @@ typedef enum {
 // forward declarations
 typedef void Value;
 typedef struct State State;
-typedef struct GCRef GCRef;
 // type defs
 /** @brief Stores an under-construction set of bindings
  *
@@ -67,8 +66,7 @@ typedef void (*PrimOpFun)(State *state, int pos, Value **args, Value *v);
 
 /** @brief Allocate a primop
  *
- * Owned by the GC
+ * Owned by the GC. Use nix_gc_decref when you're done with the pointer
- * Pass a gcref to keep a reference.
  *
  * @param[out] context Optional, stores error information
  * @param[in] fun callback
@@ -76,27 +74,23 @@ typedef void (*PrimOpFun)(State *state, int pos, Value **args, Value *v);
  * @param[in] name function name
  * @param[in] args array of argument names
  * @param[in] doc optional, documentation for this primop
- * @param[out] ref Optional, will store a reference to the returned value.
  * @return primop, or null in case of errors
  * @see nix_set_primop
  */
 PrimOp *nix_alloc_primop(nix_c_context *context, PrimOpFun fun, int arity,
-                         const char *name, const char **args, const char *doc,
+                         const char *name, const char **args, const char *doc);
-                         GCRef *ref);
 
 // Function prototypes
 
 /** @brief Allocate a Nix value
  *
- * Owned by the GC
+ * Owned by the GC. Use nix_gc_decref when you're done with the pointer
- * Pass a gcref to keep a reference.
  * @param[out] context Optional, stores error information
  * @param[in] state nix evaluator state
- * @param[out] ref Optional, will store a reference to the returned value.
  * @return value, or null in case of errors
  *
  */
-Value *nix_alloc_value(nix_c_context *context, State *state, GCRef *ref);
+Value *nix_alloc_value(nix_c_context *context, State *state);
 /** @name Getters
  */
 /**@{*/
@@ -167,27 +161,25 @@ ExternalValue *nix_get_external(nix_c_context *context, Value *);
 
 /** @brief Get the ix'th element of a list
  *
- * Pass a gcref to keep a reference.
+ * Owned by the GC. Use nix_gc_decref when you're done with the pointer
  * @param[out] context Optional, stores error information
  * @param[in] value Nix value to inspect
  * @param[in] ix list element to get
- * @param[out] ref Optional, will store a reference to the returned value.
  * @return value, NULL in case of errors
  */
 Value *nix_get_list_byidx(nix_c_context *context, const Value *value,
-                          unsigned int ix, GCRef *ref);
+                          unsigned int ix);
 /** @brief Get an attr by name
  *
- * Pass a gcref to keep a reference.
+ * Owned by the GC. Use nix_gc_decref when you're done with the pointer
  * @param[out] context Optional, stores error information
  * @param[in] value Nix value to inspect
  * @param[in] state nix evaluator state
  * @param[in] name attribute name
- * @param[out] ref Optional, will store a reference to the returned value.
  * @return value, NULL in case of errors
  */
 Value *nix_get_attr_byname(nix_c_context *context, const Value *value,
-                           State *state, const char *name, GCRef *ref);
+                           State *state, const char *name);
 
 /** @brief Check if an attribute name exists on a value
  * @param[out] context Optional, stores error information
@@ -200,6 +192,8 @@ bool nix_has_attr_byname(nix_c_context *context, const Value *value,
                          State *state, const char *name);
 
 /** @brief Get an attribute by index in the sorted bindings
+ *
+ * Owned by the GC. Use nix_gc_decref when you're done with the pointer
  * @param[out] context Optional, stores error information
  * @param[in] value Nix value to inspect
  * @param[in] state nix evaluator state
@@ -208,8 +202,7 @@ bool nix_has_attr_byname(nix_c_context *context, const Value *value,
  * @return value, NULL in case of errors
  */
 Value *nix_get_attr_byidx(nix_c_context *context, const Value *value,
-                          State *state, unsigned int i, const char **name,
+                          State *state, unsigned int i, const char **name);
-                          GCRef *ref);
 /**@}*/
 /** @name Setters
  */