[r3wretrhy]

The abort in #2415 comes from mailbox_crashdump, not from the monitor code. When a process crashes, the crash report moves the normal messages from the outer list to the inner list so they can be printed, and then puts the signals back with a plain store:

ctx->mailbox.outer_first = mailbox_process_outer_list_native(&ctx->mailbox);

Another scheduler can enqueue a signal between the detach and that store. The store overwrites it. In the repro, A monitors B twice and is killed while B crashes. A's context_destroy sends B two DemonitorSignals. If the second one arrives while B is printing its crash report, it is lost. B's context_destroy then still has a CONTEXT_MONITOR_MONITORED_LOCAL entry for A, which is already gone, so it hits assert(target != NULL). I traced this on the repro: A sent both demonitors, and B's destroy only received the first one. If B exits normal, there is no crash report and the repro never aborts.

The same line also puts the signals back oldest first, but the outer list is newest first, so context_destroy handles signals that arrived before the crash in reverse order. The comment ("now in reverse order but the process crashed anyway") assumed this didn't matter. It does matter, because those signals are still processed afterwards.

The signals now go back newest first with a compare and exchange. Anything enqueued in the meantime is detached and chained in front of them, because it is more recent. Only the owner detaches the outer list, so this can't race with another reader. Without SMP or the task driver, the store stays a plain assignment.

Testing (generic_unix, Debian 13, gcc, SMP):

• New test_mailbox_crashdump_keeps_signals in test-mailbox: it sends signals before and after a crash dump and checks that they come out in received order. It fails on release-0.7 and passes with this change. This test covers the ordering. The lost-signal race can't be reproduced deterministically in a unit test.
• monitor_race2 from the issue (20000 iterations): 9/10 runs abort on release-0.7 (1f37255). With this change, 30/30 runs return ok.
• test-erlang and test-structs pass, and test-mailbox also passes with -DAVM_DISABLE_SMP=ON. test_estdlib passed until the gen_tcp connect tests, which can't reach external hosts in my sandbox.
• clang-format-16 clean.

I based this on release-0.7 like the other fixes. Note that #2423 also adds a test to test-mailbox.c main(), so whichever lands second gets a trivial conflict there.

Fixes #2415

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

[bettio]

I think it can be merged, I just added a couple of nitpick comments. If you don't mind git rebase it on latest release-0.7 branch.

[r3wretrhy]

Thanks for the review! Rebased on latest release-0.7 (CHANGELOG conflict with #2425 only, both entries kept) and pushed 99903e2:

• The requeue helper is now mailbox_requeue_outer_signals(), declared in mailbox.h, and test_mailbox_requeue_outer_signals_after_send follows your recipe: send two signals, detach them with mailbox_process_outer_list_native, send a third, requeue, and check they come out 1, 2, 3. On the SMP build the first compare and exchange fails there, and I checked that dropping the chaining in the expected != NULL branch makes the test abort. test-mailbox passes with and without AVM_DISABLE_SMP, and clang-format 16 is clean.
• I left oldest->next = NULL; in place for symmetry with the #else branch, as you suggested.

[gabrielmancini]

Tested against the repro in #2415 (generic_unix, default build type so the assert is on, Linux x86_64):

• release-0.7 (c8fbd4c): aborted 6 of 10 runs with context_monitors_handle_terminate: Assertion `target != NULL' failed.
• This PR (720cf33): 20 of 20 runs completed, Return value: ok.

Thanks for tracking it down.