Add non-blocking native logger for NIF and CNode backends by khamilowicz · Pull Request #123 · membraneframework/unifex

khamilowicz · 2026-07-07T10:59:41Z

Summary

Adds a thread-safe, non-blocking logger (unifex_log()) that native NIF/CNode code can call to send log messages to the BEAM, forwarded through a new Unifex.Logger GenServer to Elixir's Logger. Works identically for both backends.
Unifex.Logger is opt-in (config :unifex, enable_logger: true) so consumers that never log don't pay for an idle process/thread.
Fixes the include path so any consumer can #include <unifex/logger.h> with no extra bundlex config, plus several native-side safety fixes (NULL message guard, pthread_create failure handling, retry-on-failure for the NIF fallback env).
Adds a guide (pages/logger.md) and test coverage (unit tests, an integration test project, and a wired-in integration test run).

Test plan

mix test (repo test suite, including new logger unit/integration tests and the logger_test integration project)
mix docs builds cleanly with the new guide
Verified end-to-end against a standalone example NIF project: config enables Unifex.Logger, unifex_log() calls are received and forwarded to Logger with correct formatting/metadata
CNode backend logging exercised only via a successful build, not a live end-to-end run (would need a real ei-connected CNode test harness)

🤖 Generated with Claude Code

- Made logger.h backend-agnostic by removing NIF-specific includes - Added forward declarations for UnifexEnv and UnifexPid - Created callback-based send function mechanism - Added unifex_send and unifex_get_pid_by_name to CNode backend - Created logger_nif.c with NIF-specific send implementation - Created logger_cnode.c with CNode-specific send implementation - Added convenience headers (logger_nif.h, logger_cnode.h) - Updated bundlex.exs to include backend-specific logger files - Added logger_backend.h for send function declarations The logger now works with both NIF and CNode by: 1. Using a common core implementation (logger.c) 2. Providing backend-specific send functions 3. Allowing users to register the appropriate send function for their backend Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe <vibe@mistral.ai>

Unifex.Logger no longer starts by default, since most unifex consumers never call unifex_log(); it's now enabled via `config :unifex, enable_logger: true`. Also fixes several correctness issues in the native logger: a NULL message would crash via strdup(NULL), a failed pthread_create() left a thread handle that unifex_logger_cleanup() would later join anyway (UB), and the NIF backend's fallback env was allocated once from a dlopen-time constructor with no retry, permanently disabling logging on transient allocation failure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

logger.h and logger_backend.h live outside both the nif/ and cnode/ source trees, so consumers had no portable include path to reach them (only a relative-path workaround assuming a sibling checkout worked). Add the shared directory to the unifex lib's includes so any project with `deps: [unifex: :unifex]` can `#include <unifex/logger.h>` with no extra bundlex configuration. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test_projects/logger_test's handle_load callback referenced a state struct and a handle_load_result_ok macro the spec never declared, breaking the build; both were unnecessary; the logger wires itself up automatically so no load callback is needed. Also switch its include to <unifex/logger.h> now that it's a real include path, drop the local logger.h copy that worked around its absence, and wire the project's mix test into the main repo's integration test suite. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Adds a guide covering enabling Unifex.Logger, the unifex_log() C API, where messages end up and how they're formatted, and queue/overflow behavior - verified against a working example project rather than written from the API alone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

logger_test/mix.exs was missing the deps_path/lockfile overrides that every other test_projects/* project uses, so mix test tried to fetch its own deps instead of reusing the already-fetched top-level ones, failing CI with "dependency not available". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test/fixtures/nif_ref_generated/nif/example.{c,cpp} were regenerated and committed on a machine with clang-format 22, which formats chained && differently (adds a space: "a && b") than the clang-format 14 used by CI's Ubuntu container ("a &&b"). That drift broke the "NIF test project" integration test, which compares freshly generated code against these fixtures. Reverted the affected lines to match CI's clang-format output, verified by running the test inside the membraneframeworklabs/docker_membrane image used by CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

do_logger_test_project/3 received interface but never used it (the logger_test project doesn't yet vary its assertions by NIF vs CNode), which failed mix test --warnings-as-errors in CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

FelonEkonom · 2026-07-16T16:08:59Z

+
+  @doc false
+  @spec normalize_level(atom()) :: atom()
+  def normalize_level(level) when level in @valid_levels, do: level


Why do we have to support unnormalized log levels? What would cause having such a level?

User can pass any string to unifex_log, so it is possible to break it.

We could provide an enum to specify a log level. It would remove the risk of user making a typo and the need for handling invalid log levels.

FelonEkonom · 2026-07-16T16:10:51Z

+// one of these to stay serialized via socket_mutex.
+int unifex_cnode_locked_send(UnifexEnv *env, erlang_pid *pid, char *buff,
+                             int len) {
+  pthread_mutex_lock(&env->socket_mutex);


Correct me if I am wrong, but I thought that all we have to do from the C side is to do a proper send to a proper process (the same way as original Elixir Logger works under the hood, AFAIK).

If so, why do we need any locks or mutexes in the code?

I discussed with @varsill about drawing inspiration from this Rust logger for NIFs. The library uses a queue to send messages to a GenServer in an orderly way.
Since this logger is meant to log messages executed in C, we want to avoid making it a bottleneck. The main issue with the previous implementation was that looking up the GenServer process by name was extremely time-consuming and shouldn’t happen on the main thread. Caching the PID would be unreliable, as the process could terminate at any moment. This implementation processes messages on a separate thread, ensuring it doesn’t slow down the main job.

If you discussed the shape of the implementation with @varsill, I think he should be marked as a reviewer too.
BTW what is the time difference between sending message to a process by atom name vs PID? I hear about it for the first time, is it really a big deal? I am curious 🤔

FelonEkonom · 2026-07-16T16:16:54Z

+  # The native logger queue/worker thread (see c_src/unifex/unifex/logger.c)
+  # is compiled into every unifex consumer, but only projects that actually
+  # call unifex_log() need this GenServer running to receive its messages.
+  defp logger_enabled?, do: Application.get_env(:unifex, :enable_logger, false)


Why did you make starting logger configurable? Why does it default to false?

Well, if you don't use logger in the native code, why start it? Native logger is not the main feature of unifex, so it doesn't need to be obligatory.

Ok, but this could be an another reason to avoid spawning helper logging processes

FelonEkonom · 2026-07-16T16:20:16Z

+defmodule Unifex.Logger do
+  @moduledoc """
+  Generic logger for handling log messages from C NIFs via Unifex.
+
+  This process receives log messages from C code and forwards them to Elixir's Logger.
+  It can be used by any NIF library that needs to log messages to the BEAM.
+
+  Not started by default. Enable it with:
+
+      config :unifex, enable_logger: true
+  """


Why is this module a part of public API?

This process receives log messages from C code and forwards them to Elixir's Logger. It can be used by any NIF library that needs to log messages to the BEAM.

it might be true, but why is it mentioned in docs? if somebody wants to log something, he should just call unifex_log function from C, no need to sending anything explicitly to this module

You mean that docs should be removed, right? no problem

I mean that public docs should describe public API and how our library behaves from the outside because it is what interests developers using our library. Whereas, by default, it should not be a description of how Unifex works from the inside (it may do it in cases where it make sense, in order to describe tool's outside behaviour)

FelonEkonom · 2026-07-28T11:12:03Z

        "pages/supported_types.md",
        "pages/logger.md"
      ],
+      skip_undefined_reference_warnings_on: ["pages/logger.md"],


Why do we skip undefined reference warnings? Is it because of marking Logger moduledoc as false?

FelonEkonom · 2026-07-28T11:14:48Z

+// one of these to stay serialized via socket_mutex.
+int unifex_cnode_locked_send(UnifexEnv *env, erlang_pid *pid, char *buff,
+                             int len) {
+  pthread_mutex_lock(&env->socket_mutex);


If you discussed the shape of the implementation with @varsill, I think he should be marked as a reviewer too.
BTW what is the time difference between sending message to a process by atom name vs PID? I hear about it for the first time, is it really a big deal? I am curious 🤔

FelonEkonom · 2026-07-28T11:25:21Z

+  # The native logger queue/worker thread (see c_src/unifex/unifex/logger.c)
+  # is compiled into every unifex consumer, but only projects that actually
+  # call unifex_log() need this GenServer running to receive its messages.
+  defp logger_enabled?, do: Application.get_env(:unifex, :enable_logger, false)


Ok, but this could be an another reason to avoid spawning helper logging processes

FelonEkonom · 2026-07-28T11:28:51Z

+
+  @doc false
+  @spec normalize_level(atom()) :: atom()
+  def normalize_level(level) when level in @valid_levels, do: level


We could provide an enum to specify a log level. It would remove the risk of user making a typo and the need for handling invalid log levels.

khamilowicz and others added 10 commits July 2, 2026 15:45

Logger init

13b6586

Logger Unifex rename

453bd37

Use epoch time in logger

de5dda9

Merge branch 'master' into new-logger

16cae17

Edge case fixes

e164606

Refactor of new logger

a5d5810

khamilowicz requested a review from FelonEkonom as a code owner July 7, 2026 10:59

khamilowicz force-pushed the new-logger branch from e4a584a to 7d6f3be Compare July 7, 2026 11:19

khamilowicz and others added 2 commits July 7, 2026 13:29

Formatting

000e1c7

khamilowicz force-pushed the new-logger branch from 7d6f3be to 000e1c7 Compare July 7, 2026 11:29

khamilowicz and others added 4 commits July 7, 2026 13:34

Fix strdup

d26dbae

FelonEkonom requested changes Jul 16, 2026

View reviewed changes

khamilowicz force-pushed the new-logger branch from ca4dc2a to 0c40e06 Compare July 17, 2026 10:24

khamilowicz requested a review from FelonEkonom July 17, 2026 10:25

khamilowicz force-pushed the new-logger branch from 0c40e06 to 4c1406a Compare July 17, 2026 10:26

Remove unnecessary comments

6a47df5

khamilowicz force-pushed the new-logger branch from 4c1406a to 6a47df5 Compare July 17, 2026 10:36

FelonEkonom reviewed Jul 28, 2026

View reviewed changes

FelonEkonom requested a review from varsill July 28, 2026 11:29

khamilowicz requested a review from FelonEkonom July 30, 2026 12:32

khamilowicz force-pushed the new-logger branch 2 times, most recently from 34409c3 to 309c878 Compare July 30, 2026 13:29

Use enum for logging level

ac33d64

khamilowicz force-pushed the new-logger branch from 309c878 to ac33d64 Compare July 30, 2026 13:37

Uh oh!

Conversation

khamilowicz commented Jul 7, 2026

Summary

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FelonEkonom Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FelonEkonom Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FelonEkonom Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FelonEkonom Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

FelonEkonom Jul 28, 2026 •

edited

Loading

FelonEkonom Jul 28, 2026 •

edited

Loading

FelonEkonom Jul 28, 2026 •

edited

Loading

FelonEkonom Jul 28, 2026 •

edited

Loading