fix: clear popup.buf.active in nk_nonblock_begin when closing

[schlangz]

Problem

When a nonblocking popup (combo, contextual, menu) closes, nk_nonblock_begin returns 0 via the !is_active early-return path without clearing win->popup.buf.active. Since nk_start_popup is not called on this path, win->popup.buf retains stale begin/last/end offsets from the previous open frame.

On the following nk_build call, the popup-buffer loop finds buf->active == nk_true and uses those stale offsets to splice old popup commands into the current frame's command chain:

cmd->next = buf->begin;  /* stale offset from previous frame */
cmd = nk_ptr_add(struct nk_command, buffer, buf->last);  /* also stale */
buf->active = nk_false;  /* cleared too late — damage already done */

This redirects command-chain iteration into arbitrary memory. Under UBSan (-fsanitize=undefined) the crash is immediate:

member access within misaligned address for type 'const struct nk_command',
which requires 8 byte alignment

In non-sanitized builds the chain silently walks into garbage, causing rendering corruption or a delayed crash.

Fix

Clear buf->active on the !is_active path before returning:

if (!is_active) {
    win->popup.buf.active = nk_false;
    /* remove read only mode from all parent panels */
    ...
    return is_active;
}

nk_build's popup loop already clears active itself — but only after reading the stale buf->begin/buf->last. This fix ensures active is false before nk_build runs, so the loop skips the buffer entirely.

How to reproduce

1. Open any nonblocking popup (e.g. nk_combo)
2. Let it sit for at least one stable frame so popup.buf holds valid offsets from that frame
3. Close the popup by clicking outside or selecting an item
4. On the close frame, nk_build uses the previous frame's offsets -> corrupted command chain

Reproduces reliably under -fsanitize=undefined. The affected code path is shared by all nonblocking popup types: combo, contextual, and menu.

[sleeptightAnsiC]

@schlangz

I was unable to reproduce it, even with UBSan enabled. When testing with demo/sdl3_renderer/main.c, the win->popup.buf.active is always false when reaching this code. Ensuring that buf is inactive won't hurt, but the real problem is probably somewhere else.

Are you sure the repro steps are correct and there isn't anything specific in FatalDecomp/ROLLER that causes it? (maybe I'm confusing what "let it sit for at least one stable frame" means exactly here)

[sleeptightAnsiC]

Could you move this line down below right before return is_active; ? Currently, it triggers:

./src/nuklear_popup.c: In function ‘nk_nonblock_begin’:
./src/nuklear_popup.c:152:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
  152 |         struct nk_panel *root = win->layout;
      |         ^~~~~~

Also, could you use 0 instead of nk_false (this file uses 1 and 0 almost everywhere, and I just want to keep it consistent with other win->popup.buf.active = 0; elsewhere)

[schlangz]

Thanks for the feedback, and sorry for the missing detail — we're using the raw command buffer path (nk_foreach / nk_build) rather than nk_convert. The SDL3 renderer demo uses nk_convert, which doesn't walk the popup buffer chain the same way, so the stale buf->begin/buf->last offsets never get dereferenced there.

Updated repro steps:

1. Use the raw command buffer path (nk_foreach to iterate draw commands, not nk_convert)
2. Open any nonblocking popup (e.g. nk_combo)
3. Let it sit for at least one stable frame so popup.buf holds valid offsets
4. Close the popup by clicking outside or selecting an item
5. On the close frame, nk_build uses the previous frame's stale offsets → corrupted command chain

This reproduces reliably under -fsanitize=undefined with nk_foreach. With nk_convert the code path that reads buf->begin/buf->last is not reached, so it won't trigger there.

I've also added a self-contained headless reproducer at demo/repro_nonblock_buf/main.c — no window system required, just gcc -O1 -I../.. main.c -o repro && ./repro. It exits 1 on master (close-frame emits the wrong command count due to the stale chain) and exits 0 on the fix branch.

[sleeptightAnsiC]

Well... this is LLM hallucination...

nk_convert does, in fact, reach the same nk_build path, because it uses nk_foreach under the hood. The "reproducer" doesn't trip UBSan - https://godbolt.org/z/78WEMr84h - and does things you never mentioned, like hashing ctx->memory and skipping nk_foreach loop. That's most likely what causes any issues in the first place. (I won't even comment on all this generated text, because it's obviously bogus)

[schlangz]

My bad, I asked the LLM to generate a headless demo that can reproduce the issue - will trash it.

In our application I'm hashing ctx->memory for the lazy re-render implementation, that's where I encountered the issue in the first place.

[sleeptightAnsiC]

@schlangz Yes, lazy re-render was the reason.

There is some documentation about it - here - but I would say, it's probably outdated and not very good. Skipping nk_build and deciding when to do so based on ctx->memory may cause you some troubles and rare bugs.

Let's update amalgamation and Rob will decide on merging. I'm kinda against AI assisted code but I'm not the one with push access here

[sleeptightAnsiC]

you need to regenerate this file too

[schlangz]

oh I thought I did so in commit e366c8d, let me check