Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add free / finalizer callback for void* v in nix_create_external_value #12470

Open
roberth opened this issue Feb 13, 2025 · 0 comments
Open

Add free / finalizer callback for void* v in nix_create_external_value #12470

roberth opened this issue Feb 13, 2025 · 0 comments
Labels
c api Nix as a C library with a stable interface feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc

Comments

@roberth
Copy link
Member

roberth commented Feb 13, 2025

Feature Request: Add Free Callback for void* v in nix_create_external_value

Is your feature request related to a problem?

Currently, the nix_create_external_value API takes a void* v argument, which is typically used as user data. While the returned ExternalValue* is owned by the Nix evaluator, there is no clear mechanism for managing the lifecycle of void* v. Since Nix cannot free this memory itself, one would expect to see a free callback in [NixCExternalValueDesc](https://hydra.nixos.org/build/289480061/download/1/html/structNixCExternalValueDesc.html#details), but no such function exists.

Additionally, there does not appear to be any general mechanism to detect when an external value is freed, meaning the user has no clear way to know when to clean up their allocated memory. This leads to the question:
Am I just meant to leak my void* v?

Proposed solution

A free callback should be added to NixCExternalValueDesc to allow proper cleanup of void* v when an external value is garbage collected. This would ensure that resources are not leaked unnecessarily.

Implement a mechanism for registering finalizers in mkExternal, so that destructors may be called.

Since Nix uses a conservative garbage collector, finalizers are not always guaranteed to run before process exit. Even so, adding a free callback would improve resource management in most cases. This should be documented.

To ensure correctness, a test utility should be introduced to force finalization, allowing unit tests to verify that the free callback is correctly handled.

Alternative solutions

  • Document the current behavior explicitly to warn users about potential memory leaks.

Additional context

This issue was initially raised by sodiboo on Matrix. Investigation shows that while ExternalValueBase has a virtual destructor, it is never called because Value relies entirely on GC, and mkExternal does not register a finalizer. This means that external values are never explicitly freed, leading to potential memory leaks.

If Nix were to introduce finalization in the future, it could start invoking code written by C API consumers that has never run before. If finalization is not fully implemented and tested, this could result in unexpected crashes when untested code starts executing. Ensuring that finalization is tested from the start would mitigate this risk.


Add 👍 to issues you find important.

@roberth roberth added c api Nix as a C library with a stable interface feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc labels Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c api Nix as a C library with a stable interface feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc
Projects
None yet
Development

No branches or pull requests

1 participant