diff --git a/lib/dev.ml b/lib/dev.ml index b13b8e1..d920262 100644 --- a/lib/dev.ml +++ b/lib/dev.ml @@ -3040,9 +3040,13 @@ static jmp_buf program_return; /* main()'s frame, from anywhere */ * instant after closing it (POSIX hands out the lowest free descriptor, so the * compiler thread's next socket would have become this process's stdout, and * the next llc would have inherited it) goes away with the close that caused - * it: fd 1 is never free. */ + * it: fd 1 is never free. + * + * Nothing is flushed here either, and that is the same decision the park makes + * one function down: a flush of a pipe nobody is reading is an unbounded wait, + * and every line of it would be a line the compiler spends still believing the + * program is running. [flan_merged_park] flushes once it has said otherwise. */ static void flan_merged_exit(int32_t status) { - fflush(NULL); program_status = status; longjmp(program_return, 1); } @@ -3055,10 +3059,35 @@ static void flan_merged_exit(int32_t status) { * * [while], not [if]: a condition variable may wake a waiter that nobody * signalled, and [program_asked] is the fact — the wakeup is only a hint that - * it is worth looking again. */ + * it is worth looking again. + * + * THE STATE IS FLIPPED BEFORE ANYTHING IS FLUSHED, and the order is the whole + * of a fix. stdout is a 64K pipe into this process, drained by the compiler + * thread, which stops draining for as long as it is answering a request. A + * program that printed as it ran leaves that pipe full when it finishes, so + * the flush below can wait for a reader that is busy — and every moment it + * waits is a moment [flan_merged_program_state] still answers RUNNING about a + * program that is over. Close a window, press the key that runs it again, and + * the answer was "the program is already running": the request itself was what + * kept the reader from draining. The state flip is two stores under a lock and + * cannot block on anything, so it goes first and the truth is available + * immediately; the flush and the notice follow, and they are courtesies. + * + * What that widens is the window in which the program is PARKED and not yet + * waiting. Nothing is lost in it: [flan_merged_rerun] sets [program_asked] and + * signals under the same lock, a signal delivered to nobody is discarded, and + * the [while] below reads the flag rather than the wakeup — so a request that + * lands in the window is taken and the wait falls straight through. The one + * visible cost is that two re-runs arriving in that window are both answered + * "ok" for a single run. That race existed before and was a microsecond wide; + * it is now as wide as a flush, which is the right trade against a refusal + * that was simply false. */ static void flan_merged_park(void) { flan_condition_stacks_reset(); if (flan_dev_frames_reset) flan_dev_frames_reset(); + pthread_mutex_lock(&program_lock); + program_state = PROGRAM_PARKED; + pthread_mutex_unlock(&program_lock); fflush(NULL); fprintf(stderr, "flan dev: the program finished with %d; the process is parked and " @@ -3066,7 +3095,6 @@ static void flan_merged_park(void) { (int)program_status); fflush(stderr); pthread_mutex_lock(&program_lock); - program_state = PROGRAM_PARKED; while (!program_asked) pthread_cond_wait(&program_wake, &program_lock); program_asked = 0; program_state = PROGRAM_RUNNING; @@ -3081,10 +3109,12 @@ static void flan_merged_park(void) { * * [flan_merged_rerun] refuses a program that is already running rather than * remembering the request, and that refusal is the only one there can be: the - * test and the signal are under the same lock, so a request that arrives in - * the microsecond between the finished run's longjmp and the park is either - * seen as running (refused, and the program parks a moment later) or seen as - * parked (taken). Queueing it instead would mean a second main starting the + * test and the signal are under the same lock, so a request that arrives + * between the finished run's longjmp and the park is either seen as running + * (refused, and the program parks a moment later) or seen as parked (taken). + * The park flips the state before it flushes, so the second is now the usual + * answer rather than the lucky one, and what that widens is written out + * there. Queueing it instead would mean a second main starting the * instant the first finished, which is never what somebody pressing a key * meant. */ int flan_merged_rerun(void) { diff --git a/runtime/flan_rt.c b/runtime/flan_rt.c index 3105bad..60e274b 100644 --- a/runtime/flan_rt.c +++ b/runtime/flan_rt.c @@ -199,11 +199,21 @@ void flan_write_stdout(const uint8_t *p, int64_t n) { * That is fine when the program is its own process and wrong when the compiler * is in the same one — [exit] would take the session down with the program. * The merged entry point installs a hook that flushes, tells the compiler the - * program is done, and parks instead. See lib/dev.ml. */ + * program is done, and parks instead. See lib/dev.ml. + * + * There is no fflush before the hook, and its absence is the fix to a hang + * rather than a saving. [exit] flushes every stream itself, so the call was + * doing nothing at all for a program that is its own process; the only path it + * ever ran on was the hook's. And on that path stdout is a 64K pipe into the + * compiler thread, which is not reading it while it is answering a request — + * so this flush was the first thing to block when a printing program finished, + * and it blocked *before* the hook could say the program had. The compiler + * then answered "the program is already running" to the one key the person + * had just pressed to run it again. The hook flushes after it has said so; + * see flan_merged_park in lib/dev.ml, which carries the rest of the argument. */ void (*flan_exit_hook)(int32_t status) = 0; void flan_exit(int32_t status) { - fflush(stdout); if (flan_exit_hook) flan_exit_hook(status); /* does not return */ exit((int)status); }