diff --git a/lib/dev.ml b/lib/dev.ml index d920262..addaabe 100644 --- a/lib/dev.ml +++ b/lib/dev.ml @@ -3146,9 +3146,15 @@ int flan_merged_program_state(void) { * message has already sent two investigations in this repository to the wrong * place. Gone is the honest state, and the client already has words for it. * - * [atexit] covers exit(3), which is what flan_rt.c's rt_die uses. _exit and - * abort skip it by design, so the places that take those routes unlink for - * themselves; see die_now in flan_agent.c. */ + * [atexit] covers exit(3), which is the compiler thread's own way out and the + * way out of a program that somehow returns from main. It does NOT cover the + * two places a program dies where it stands: rt_die in flan_rt.c and die_now + * in flan_agent.c both take _exit, because the atexit chain and the ELF + * destructors want the loader lock a dlopening listener thread may be holding + * — and in this build that chain also holds OCaml's shutdown. Both of them + * therefore unlink this path by hand, which is why this function's body is + * written out three times in the tree rather than shared: it is four lines, and + * the alternative is a runtime that has to link against the daemon. */ static void flan_merged_unlink_sock(void) { const char *s = getenv("FLAN_DEV_SOCK"); if (s != NULL && *s != '\0') unlink(s); diff --git a/runtime/flan_rt.c b/runtime/flan_rt.c index 60e274b..a1190f5 100644 --- a/runtime/flan_rt.c +++ b/runtime/flan_rt.c @@ -158,8 +158,11 @@ void flan_rt_init(int32_t argc, char **argv) { } /* Defined below with the rest of the non-local exits, and forward-declared - * here because the argument vector is built long before them. */ + * here because the argument vector is built long before them. [rt_flush_out] + * is the flush every one of those paths does first; its own note says why it + * is not [fflush(stdout)]. */ static _Noreturn void rt_die(void); +static void rt_flush_out(void); /* The one malloc in this file that is not an allocator's, because the argument * vector belongs to the process rather than to any region a Flan program named. @@ -172,7 +175,7 @@ void flan_argv(flan_slice *out) { if (rt_args == NULL && rt_argc > 0) { rt_args = (flan_slice *)malloc(sizeof(flan_slice) * (size_t)rt_argc); if (rt_args == NULL) { - fflush(stdout); + rt_flush_out(); fprintf(stderr, "flan: out of memory building the argument vector for %d " "arguments\n", @@ -391,15 +394,67 @@ void flan_escape_bytes(const uint8_t *p, int64_t n, flan_slice *out) { * redirected stdout is not, so without this the error appears above the output * that led to it. */ +#include +#include + +/* That flush, made incapable of waiting. + * + * Under a merged `flan dev' stdout is a 64K pipe into the compiler thread, + * which is not reading it while it is answering a request. A trap taken by a + * program that had filled that pipe therefore began by blocking in the flush + * meant to order its own last words — and a bounds failure that hangs instead + * of dying is the worst shape a trap can take, because the person watching has + * no message and no exit status and no reason to think anything happened. + * + * So fd 1 is put into non-blocking mode first and the flush is best-effort. + * What that costs is a truncated tail: whatever no longer fits in the pipe is + * dropped rather than waited for. What it buys is that the trap always reaches + * its message and its exit. On a terminal, a file, or a pipe with room — + * which is every run that is not this one pathological case — O_NONBLOCK + * changes nothing at all, and the acceptance corpus diffs this output. + * + * The return value is ignored deliberately, twice over: a failed fcntl leaves + * the old blocking behaviour, which is what this code did before, and a flush + * that reports EAGAIN has done as much as it is going to. There is nothing a + * dying process can do about either. */ +static void rt_flush_out(void) { + int flags = fcntl(1, F_GETFL, 0); + if (flags >= 0) (void)fcntl(1, F_SETFL, flags | O_NONBLOCK); + (void)fflush(stdout); +} + +/* [_exit] and not [exit], for the reason die_now gives in + * vendor/agent/flan_agent.c and gives at length: this runs on the game thread, + * the dev agent's listener thread may be inside [dlopen] holding the loader + * lock, and [exit] runs the atexit chain and the ELF destructors, which want + * that same lock. In a merged build that chain also holds OCaml's shutdown, + * and it would be run from a thread that is not OCaml's. A trap that deadlocks + * in the runtime's teardown is the same failure as a trap that hangs on a full + * pipe, reached a few instructions later. + * + * What [_exit] skips is the atexit handler that removes the editor's socket — + * so, exactly as die_now does, this removes it by hand. A socket file left on + * disk with nothing accepting on it answers the next client with + * ECONNREFUSED, which reads like a daemon that is there and refusing rather + * than one that died; that message has already sent two investigations in this + * repository to the wrong place. FLAN_DEV_SOCK is unset in an ordinary run and + * then this does nothing. + * + * The two functions are kept saying the same thing on purpose. They are the + * two ways a Flan program dies where it stands, and a difference between them + * would be a difference nobody could predict from the outside. */ static _Noreturn void rt_die(void) { - fflush(stdout); + const char *sock; + rt_flush_out(); fflush(stderr); - exit(134); + sock = getenv("FLAN_DEV_SOCK"); + if (sock != NULL && *sock != '\0') unlink(sock); + _exit(134); } _Noreturn void flan_bounds_fail(const uint8_t *loc, int64_t loclen, int64_t idx, int64_t len) { - fflush(stdout); + rt_flush_out(); fprintf(stderr, "%.*s: index %lld is out of bounds for length %lld\n", (int)loclen, (const char *)loc, (long long)idx, (long long)len); rt_die(); @@ -458,7 +513,7 @@ void flan_error(uint32_t type_id, void *condition, void *xfer, flan_break_hook(name, namelen, condition, xfer); if (*(void **)xfer != NULL) return; } - fflush(stdout); + rt_flush_out(); fprintf(stderr, "unhandled %.*s\n", (int)namelen, (const char *)name); rt_die(); } @@ -468,7 +523,7 @@ void flan_error(uint32_t type_id, void *condition, void *xfer, * there is nowhere to resume, so there is nothing else to do. */ _Noreturn void flan_restart_fail(const uint8_t *loc, int64_t loclen, const uint8_t *name, int64_t namelen) { - fflush(stdout); + rt_flush_out(); fprintf(stderr, "%.*s: no restart named %.*s is active\n", (int)loclen, (const char *)loc, (int)namelen, (const char *)name); rt_die(); @@ -483,7 +538,7 @@ _Noreturn void flan_restart_args_fail(const uint8_t *loc, int64_t loclen, const uint8_t *name, int64_t namelen, const uint8_t *want, int64_t wantlen, const uint8_t *got, int64_t gotlen) { - fflush(stdout); + rt_flush_out(); fprintf(stderr, "%.*s: restart %.*s takes %.*s, given %.*s\n", (int)loclen, (const char *)loc, (int)namelen, (const char *)name, (int)wantlen, (const char *)want, (int)gotlen, (const char *)got); @@ -498,7 +553,7 @@ _Noreturn void flan_restart_args_fail(const uint8_t *loc, int64_t loclen, _Noreturn void flan_restart_unarmed(const uint8_t *loc, int64_t loclen, const uint8_t *name, int64_t namelen, const uint8_t *want, int64_t wantlen) { - fflush(stdout); + rt_flush_out(); fprintf(stderr, "%.*s: restart %.*s takes %.*s, and whatever took it supplied no " "arguments — a restart with parameters cannot be taken from the " @@ -514,7 +569,7 @@ _Noreturn void flan_restart_unarmed(const uint8_t *loc, int64_t loclen, * lexical case is refused by the checker; this is the one that reaches a * function through a call, where nothing static could see it. */ _Noreturn void flan_transfer_fail(const uint8_t *loc, int64_t loclen) { - fflush(stdout); + rt_flush_out(); fprintf(stderr, "%.*s: a defer invoked a restart, which a defer may not do — it is " "the cleanup a transfer runs on its way out\n", @@ -524,7 +579,7 @@ _Noreturn void flan_transfer_fail(const uint8_t *loc, int64_t loclen) { _Noreturn void flan_slice_fail(const uint8_t *loc, int64_t loclen, int64_t lo, int64_t hi, int64_t len) { - fflush(stdout); + rt_flush_out(); fprintf(stderr, "%.*s: slice [%lld %lld) is out of bounds for length %lld\n", (int)loclen, (const char *)loc, (long long)lo, (long long)hi, (long long)len); @@ -632,7 +687,7 @@ void flan_slice_error(const uint8_t *loc, int64_t loclen, int64_t lo, * testing high <= length would wave the failure through. */ _Noreturn void flan_slice_promise_fail(const uint8_t *loc, int64_t loclen, int64_t n) { - fflush(stdout); + rt_flush_out(); fprintf(stderr, "%.*s: slice-from-ptr was promised %lld elements behind the pointer, " "and a count of elements is never negative\n", @@ -706,7 +761,7 @@ static const uint8_t flan_arith_name[] = "ArithError"; * formatting is the unhandled path's job, and this is the unhandled path. */ static void flan_arith_fail(const uint8_t *loc, int64_t loclen, int32_t op, int64_t lhs, int64_t rhs) { - fflush(stdout); + rt_flush_out(); switch (op) { case FLAN_ARITH_DIV_ZERO: fprintf(stderr, "%.*s: divide by zero: (/ %lld 0)\n", (int)loclen, @@ -1206,7 +1261,7 @@ void flan_alloc_free_all(flan_allocator *a, const uint8_t *loc, int64_t loclen) } _Noreturn void flan_null_alloc_fail(const uint8_t *loc, int64_t loclen) { - fflush(stdout); + rt_flush_out(); fprintf(stderr, "%.*s: this allocator is null — a zeroed Allocator was never given " "one\n", @@ -1215,7 +1270,7 @@ _Noreturn void flan_null_alloc_fail(const uint8_t *loc, int64_t loclen) { } _Noreturn void flan_free_all_fail(const uint8_t *loc, int64_t loclen) { - fflush(stdout); + rt_flush_out(); fprintf(stderr, "%.*s: this allocator does not offer free-all — it has no region to " "release, and releasing nothing is not the same as releasing " @@ -1271,7 +1326,7 @@ void flan_alloc_region_only(flan_allocator *a, const uint8_t *loc, } _Noreturn void flan_region_only_fail(const uint8_t *loc, int64_t loclen) { - fflush(stdout); + rt_flush_out(); fprintf(stderr, "%.*s: this container's elements own storage, and this allocator can " "free one block — so a free here would release the slots and leak " @@ -1349,7 +1404,7 @@ int64_t flan_alloc_id(flan_allocator *a) { return (int64_t)(intptr_t)a; } _Noreturn void flan_vec_stale_fail(const uint8_t *loc, int64_t loclen, int64_t was, int64_t now) { - fflush(stdout); + rt_flush_out(); fprintf(stderr, "%.*s: this container's allocator was released — it was made at " "epoch %lld and the allocator is at %lld now\n", @@ -1359,7 +1414,7 @@ _Noreturn void flan_vec_stale_fail(const uint8_t *loc, int64_t loclen, _Noreturn void flan_vec_bounds_fail(const uint8_t *loc, int64_t loclen, int64_t i, int64_t len) { - fflush(stdout); + rt_flush_out(); fprintf(stderr, "%.*s: index %lld is out of bounds for length %lld\n", (int)loclen, (const char *)loc, (long long)i, (long long)len); rt_die(); @@ -2852,7 +2907,7 @@ int64_t flan_file_fail_reason(void) { return flan_file_fail; } * so this traps naming the declare-c that was called, the way an out-of-bounds * index traps naming its site. See lib/shim.ml, which emits the call. */ _Noreturn void flan_shim_nul_fail(const char *site) { - fflush(stdout); + rt_flush_out(); fprintf(stderr, "%s: a string passed to C contains a NUL byte — C reads to the " "first one, so the value this function would act on is a prefix of "