diff --git a/docs/api/bytes.html b/docs/api/bytes.html index 6147d4451..348efda53 100644 --- a/docs/api/bytes.html +++ b/docs/api/bytes.html @@ -324,7 +324,7 @@

from_ptr fn

string_from_ptr fn

string_from_ptr(src, length) -> string

src: ptr, length: int

-

One-call ptr -> string crossing: a fresh refcounted binary string copying `length` bytes from a raw pointer `src`, without an intermediate bytes. The direct answer to "I have a ptr + len, I want a string" (issue #1782).

+

One-call ptr -> string crossing: a fresh refcounted binary string copying `length` bytes from a raw pointer `src`, without an intermediate bytes. The direct answer to "I have a ptr + len, I want a string" (issue #1782). `src` must be a RAW data pointer: `bytes.data(b)` or a C buffer. A bytes handle is also a `ptr` but is refused (panic): for a handle use `bytes.to_string(b, n)`.

view fn

diff --git a/docs/api/http.html b/docs/api/http.html index 1ac2b40ae..b8b90d0ce 100644 --- a/docs/api/http.html +++ b/docs/api/http.html @@ -451,6 +451,7 @@

server_port extern

server_start_raw extern

server_start_raw(server) -> int

server: ptr

+

A start of a server that is already running is refused (-1, the reason in http_server_error).

server_set_host fn

@@ -467,6 +468,7 @@

server_start_background_raw extern<

server_stop extern

server_stop(server)

server: ptr

+

Stop waits until the server's run is over, whichever start began it and on whatever thread, so a free may follow at once. On one of the server's own threads (a handler, the monitor) it only signals, and a free there is fatal: free it from the thread that started it.

server_free extern

@@ -795,7 +797,7 @@

server_start fn

server_start_background fn

- 
server_start_background(server) -> string
+ 
server_start_background(server)

server: ptr

Start a server on a detached native thread. Useful for integration tests and embedding hosts that need multiple local servers in one process without tying up the Aether actor scheduler.

diff --git a/docs/http-server.md b/docs/http-server.md index 57b6caae2..9130815ad 100644 --- a/docs/http-server.md +++ b/docs/http-server.md @@ -913,6 +913,19 @@ any connection; a keep-alive connection waiting for its client's next request, or parked, is closed at once rather than after its idle timeout. +A stop waits for the server's run to be over, whichever start began it +and whichever thread it runs on (the caller's for `server_start`, an +actor's, or the server's own for `server_start_background`): when +`http_server_stop` returns, the accept loop, its workers and the +parking lot are done, so the server can be freed at once, or started +again. Called on one of the server's own threads (a handler, the +monitor), a stop cannot wait for a run that waits for that thread, so +it only signals, and the run ends once the call returns; +`http_server_free` there is a fatal error, since the run still uses +the server: free it from the thread that started it, whose own stop +waits. A start of a server that is already running is refused, with +the reason in `http_server_error`. + --- ## Embedded / background servers @@ -920,10 +933,7 @@ timeout. `http_server_start_background_raw(server)` runs the accept loop on a thread of its own and returns immediately, so a host can keep serving while doing other work (and stop it later with `http_server_stop`). -Stop waits for that thread: when it returns, the accept loop, its -workers and the parking lot are done, so the server can be freed at -once, and another started in the same process. This is the *embedded* -mode and behaves like a library, not a CLI: +This is the *embedded* mode and behaves like a library, not a CLI: - **No banner.** It has no monitor unless the program gives it one, so it prints nothing: an embedded server is controlled by diff --git a/new_changelogs/20261109T185326Z-fixed-http-server-stop-waits-foreground.md b/new_changelogs/20261109T185326Z-fixed-http-server-stop-waits-foreground.md new file mode 100644 index 000000000..7f7703538 --- /dev/null +++ b/new_changelogs/20261109T185326Z-fixed-http-server-stop-waits-foreground.md @@ -0,0 +1,14 @@ +- **`http_server_stop` waits for a foreground server too, so a free after it + is safe.** Stop waited only for a server started with + `http_server_start_background_raw`. A foreground `http_server_start_raw` + run on another thread (an actor's) was signalled and left running, and the + `http_server_free` that followed released it under its accept loop and a + pool worker still serving a keep-alive connection: a segfault in + `conn_recv_request_start` / `http_resolve_route` on macOS and Linux (found + by servirtium-vcr). Now a stop waits until the run is over, whichever start + began it. On one of the server's own threads (a handler, the monitor, an + HTTP/2 dispatch worker, the proxy driver's middleware) it only signals, + since the run waits for that thread: a background server stopped from a + handler used to hang there. `http_server_free` on such a thread while the + server runs is a fatal error rather than a use after free, and a start of + a running server is refused with the reason in `http_server_error`. diff --git a/new_changelogs/20261109T200443Z-fixed-macos-deployment-target-follows-os.md b/new_changelogs/20261109T200443Z-fixed-macos-deployment-target-follows-os.md new file mode 100644 index 000000000..961b54dcc --- /dev/null +++ b/new_changelogs/20261109T200443Z-fixed-macos-deployment-target-follows-os.md @@ -0,0 +1 @@ +- **`ae build` on macOS no longer prints an `ld: warning` for every Homebrew dylib it links.** With `MACOSX_DEPLOYMENT_TARGET` unset, the toolchain targeted the installed SDK's macOS, which can trail the running OS; Homebrew builds its dylibs for the running OS, so each link of a program using nghttp2 (or any newer-built dylib) warned "building for macOS-27.0, but linking with dylib ... built for newer version 27.2", and the warning landed in the output of `ae run`. A native `ae build` now defaults the deployment target to the running OS; a `MACOSX_DEPLOYMENT_TARGET` you set still wins, and cross builds are unchanged. `ae cflags --libs` adds the same `-mmacosx-version-min=` for a build that drives its own compiler. diff --git a/new_changelogs/20261109T200545Z-fixed-bytes-string-from-ptr-handle.md b/new_changelogs/20261109T200545Z-fixed-bytes-string-from-ptr-handle.md new file mode 100644 index 000000000..52459e8b8 --- /dev/null +++ b/new_changelogs/20261109T200545Z-fixed-bytes-string-from-ptr-handle.md @@ -0,0 +1 @@ +- **`bytes.string_from_ptr` refuses a bytes handle instead of copying its header** (`std/bytes/aether_bytes.c`, `std/bytes/module.ae`). `string_from_ptr(src, n)` wants a raw data pointer (`bytes.data(b)` or a C buffer), but a bytes handle is also a `ptr`, so `bytes.string_from_ptr(b, 4)` compiled and silently returned the `AetherBytes` struct header as text (it produced garbage keys in servirtium-vcr's Azure Key Vault test). `AetherBytes` now carries an 8-byte tag (cleared on free) and `string_from_ptr` panics on a live handle with a message naming `bytes.to_string(b, n)` and `bytes.data(b)`; the check reads one byte of a raw buffer unless the tag matches, so short C buffers are never over-read. The module comment, `std/bytes/README.md` and `docs/api` say `src` must be a raw data pointer. Test: `tests/regression/test_bytes_string_from_ptr_handle.ae` (the handle is refused; `to_string` and `string_from_ptr(bytes.data(b), n)` stay binary-safe over NUL and 0xff). diff --git a/std/bytes/README.md b/std/bytes/README.md index d80dedcf5..1c3e2dff5 100644 --- a/std/bytes/README.md +++ b/std/bytes/README.md @@ -42,7 +42,7 @@ There is deliberately no `ptr as string` cast (they are different types). ```aether,fragment // ptr -> string (I have a raw buffer + length, I want a binary string) -s = bytes.string_from_ptr(p, n) +s = bytes.string_from_ptr(p, n) // p: bytes.data(b) or a C buffer, NOT a bytes handle // string -> ptr (I have a string, I want a raw pointer to walk) n = string.length(s) @@ -52,6 +52,12 @@ p = bytes.data(b) // ... use p ... (keep `b` alive as long as you use `p`) ``` +`src` must be a raw data pointer. A bytes handle is also a `ptr`, so the +compiler cannot tell the two apart, but `string_from_ptr` recognises a handle +and panics rather than copy its header as text. To turn a handle into a +string use `bytes.to_string(b, n)`; to use the handle's payload as a raw +pointer, `bytes.string_from_ptr(bytes.data(b), n)`. + The longhand still works and is useful when you build a buffer incrementally: ```aether,fragment diff --git a/std/bytes/aether_bytes.c b/std/bytes/aether_bytes.c index f65f465b8..d0fdb1733 100644 --- a/std/bytes/aether_bytes.c +++ b/std/bytes/aether_bytes.c @@ -2,6 +2,7 @@ #include "../string/aether_string.h" #include "../../runtime/aether_resource_caps.h" #include "../../runtime/utils/aether_wipe.h" +#include "../../runtime/actors/aether_panic.h" #include #include @@ -16,11 +17,33 @@ * AetherBytes would need length == 0xAE57C0DE (~2.9GB) to collide, and the * header plausibility checks inside is_aether_string reject that too. */ struct AetherBytes { + /* Handle tag, so a bytes HANDLE handed to aether_bytes_string_from_ptr + * (which wants the raw data pointer) can be told from a data pointer. + * Cleared on free. */ + unsigned char tag[8]; size_t length; size_t capacity; char* data; }; +static const unsigned char BYTES_TAG[8] = {0xAE, 'B', 'Y', 'T', 'E', 'S', 0x01, 0xFF}; + +/* Is `p` a live AetherBytes handle? Compares byte by byte with a short + * circuit, so a raw buffer whose first byte differs (the near-certain case) is + * read for exactly one byte and a short allocation is never over-read. A full + * tag match is then checked for a plausible header. */ +static int is_aether_bytes(const void* p) { + if (!p) return 0; + const unsigned char* q = (const unsigned char*)p; + for (int i = 0; i < 8; i++) { + if (q[i] != BYTES_TAG[i]) return 0; + } + const AetherBytes* b = (const AetherBytes*)p; + if (b->capacity < b->length) return 0; + if (b->capacity > 0 && !b->data) return 0; + return 1; +} + /* Grow the underlying buffer so it has at least `min_capacity` bytes. * Doubles each time to keep amortised-O(1) append cost; falls back to * `min_capacity` itself once doubling stops being enough. Returns 1 @@ -59,6 +82,7 @@ AetherBytes* aether_bytes_new(int initial_capacity) { if (initial_capacity < 0) return NULL; AetherBytes* b = (AetherBytes*)aether_caps_malloc(sizeof(AetherBytes)); if (!b) return NULL; + memcpy(b->tag, BYTES_TAG, sizeof b->tag); b->length = 0; b->capacity = 0; b->data = NULL; @@ -92,6 +116,13 @@ AetherBytes* aether_bytes_from_ptr(void* src, int length) { * I want a length-aware binary string". */ void* aether_bytes_string_from_ptr(void* src, int length) { if (length < 0) return NULL; + /* `src` is the raw DATA pointer (bytes.data(b), or a C buffer). A bytes + * handle is also a `ptr`, so the type system cannot tell; copying it would + * yield the struct header as text. Refuse it loudly (the std style for a + * misuse that has no failure value to return: aether_panic). */ + if (is_aether_bytes(src)) + aether_panic("bytes.string_from_ptr: `src` is a bytes handle, not a raw data pointer; " + "use bytes.to_string(b, n), or pass bytes.data(b)"); if (length == 0 || !src) return (void*)string_new_with_length("", 0); return (void*)string_new_with_length((const char*)src, (size_t)length); } @@ -470,6 +501,7 @@ void aether_bytes_free(AetherBytes* b) { * so the resource accounting balances the aether_caps_malloc / * aether_caps_realloc above. */ aether_caps_free(b->data, b->capacity); + memset(b->tag, 0, sizeof b->tag); aether_caps_free(b, sizeof(AetherBytes)); } diff --git a/std/bytes/module.ae b/std/bytes/module.ae index e6aa8d579..24bfce92e 100644 --- a/std/bytes/module.ae +++ b/std/bytes/module.ae @@ -160,6 +160,9 @@ from_ptr(src: ptr, length: int) -> ptr { // One-call ptr -> string crossing: a fresh refcounted binary string copying // `length` bytes from a raw pointer `src`, without an intermediate bytes. // The direct answer to "I have a ptr + len, I want a string" (issue #1782). +// `src` must be a RAW data pointer: `bytes.data(b)` or a C buffer. A bytes +// handle is also a `ptr` but is refused (panic): for a handle use +// `bytes.to_string(b, n)`. string_from_ptr(src: ptr, length: int) -> string { return aether_bytes_string_from_ptr(src, length) } diff --git a/std/http/module.ae b/std/http/module.ae index 858f86db1..a0be569aa 100644 --- a/std/http/module.ae +++ b/std/http/module.ae @@ -274,8 +274,14 @@ extern http_server_port(server: ptr) -> int // loopback binds. A host name binds the IPv4 address it resolves to, // and one that does not resolve fails the bind (#2639). extern http_server_set_host(server: ptr, host: string) +// A start of a server that is already running is refused (-1, the reason in +// http_server_error). extern http_server_start_raw(server: ptr) -> int extern http_server_start_background_raw(server: ptr) -> int +// Stop waits until the server's run is over, whichever start began it and on +// whatever thread, so a free may follow at once. On one of the server's own +// threads (a handler, the monitor) it only signals, and a free there is fatal: +// free it from the thread that started it. extern http_server_stop(server: ptr) extern http_server_free(server: @frees ptr) // TLS termination (#260 Tier 0). Loads the cert+key (PEM-encoded); @@ -872,7 +878,7 @@ server_start(server: ptr) -> { server_start_background(server: ptr) -> { rc = http_server_start_background_raw(server) if rc < 0 { - return "server background start failed" + return server_error_or(server, "server background start failed") } return "" } diff --git a/std/http/server/h2/aether_h2.c b/std/http/server/h2/aether_h2.c index e425209b0..203181130 100644 --- a/std/http/server/h2/aether_h2.c +++ b/std/http/server/h2/aether_h2.c @@ -604,8 +604,12 @@ static void h2_task_run(void* arg) { /* Route table + middleware are read-only after startup, so running the * handler here (off the connection thread) is safe across any number of * sessions; nghttp2_session is NEVER touched here. Each task owns its - * own (req, res). */ + * own (req, res). The worker is one of the server's own threads for the + * handler: its connection thread waits for this task, so a stop from the + * handler must not wait for the run. */ + http_server_thread_enter(sess->server); http_server_dispatch_for_h2(sess->server, task->req, task->res); + http_server_thread_leave(sess->server); /* Push to the session's ready queue + wake its connection thread. The * session's in_flight counter (bumped before submit) keeps `sess` alive diff --git a/std/net/aether_http_evloop.c b/std/net/aether_http_evloop.c index 50434157f..168fb1e85 100644 --- a/std/net/aether_http_evloop.c +++ b/std/net/aether_http_evloop.c @@ -1991,6 +1991,10 @@ static void* ev_driver_main(void* arg) { EvDriver* d = (EvDriver*)arg; AetherIoEvent events[64]; + /* One of the server's own threads while the run lasts: the middleware + * ahead of the proxy runs here, and one that stops the server must not + * wait for the run, which joins this thread. */ + http_server_thread_enter(d->loop->server); aether_io_poller_add(&d->poller, d->wake_r, NULL, AETHER_IO_READ); while (!atomic_load(&d->loop->stopping)) { @@ -2041,6 +2045,7 @@ static void* ev_driver_main(void* arg) { EvConn* c = d->by_fd[fd]; if (c && c->client_fd == fd) ev_conn_close(d, c); } + http_server_thread_leave(d->loop->server); return NULL; } diff --git a/std/net/aether_http_pool.c b/std/net/aether_http_pool.c index 29c62a3f7..6d2e2e690 100644 --- a/std/net/aether_http_pool.c +++ b/std/net/aether_http_pool.c @@ -130,6 +130,10 @@ int http_pool_default_workers(void) { static void* http_pool_worker(void* arg) { HttpConnectionPool* pool = (HttpConnectionPool*)arg; + /* One of the server's own threads while the run lasts: a handler here + * that stops the server must not wait for the run, which joins this + * thread. */ + http_server_thread_enter(pool->server); while (1) { pthread_mutex_lock(&pool->lock); while (pool->count == 0 && !pool->shutdown) { @@ -151,6 +155,7 @@ static void* http_pool_worker(void* arg) { else http_server_drain_connection(pool->server, item.fd); atomic_fetch_sub(&http_pool_busy_workers, 1); } + http_server_thread_leave(pool->server); return NULL; } diff --git a/std/net/aether_http_server.c b/std/net/aether_http_server.c index 3619ffda3..8241c774f 100644 --- a/std/net/aether_http_server.c +++ b/std/net/aether_http_server.c @@ -149,6 +149,8 @@ const char* http_server_set_keepalive_raw(HttpServer* s, int e, int m, int64_t i (void)s; (void)e; (void)m; (void)i; return "keep-alive unavailable: networking not built in"; } void http_server_drain_connection(HttpServer* s, int fd) { (void)s; (void)fd; } +void http_server_thread_enter(HttpServer* s) { (void)s; } +void http_server_thread_leave(HttpServer* s) { (void)s; } const char* http_server_shutdown_graceful_raw(HttpServer* s, int64_t t) { (void)s; (void)t; return ""; } const char* http_server_set_health_probes_raw(HttpServer* s, const char* lp, const char* rp, HttpReadyCheck rc, void* ud) { @@ -1486,6 +1488,12 @@ HttpServer* http_server_create(int port) { pthread_mutex_init(&server->live_lock, NULL); pthread_mutex_init(&server->tls_lock, NULL); pthread_mutex_init(&server->tls_config_lock, NULL); + pthread_mutex_init(&server->run_lock, NULL); + pthread_cond_init(&server->run_done, NULL); + server->serving = 0; + server->server_tids = NULL; + server->server_tid_count = 0; + server->server_tid_cap = 0; server->live_fds = NULL; server->live_count = 0; server->live_cap = 0; @@ -7521,22 +7529,94 @@ static void* accept_thread_fn(void* arg) { static int http_server_run(HttpServer* server); static int http_server_serve(HttpServer* server); -int http_server_start_raw(HttpServer* server) { +/* The thread is one of the server's own for the run (see server_tids). A + * thread that could not say so would have a stop on it wait for itself, so + * running out of memory here is fatal rather than quietly skipped. */ +void http_server_thread_enter(HttpServer* server) { + pthread_mutex_lock(&server->run_lock); + if (server->server_tid_count == server->server_tid_cap) { + int cap = server->server_tid_cap ? server->server_tid_cap * 2 : 16; + aether_tid_t* grown = realloc(server->server_tids, (size_t)cap * sizeof(aether_tid_t)); + if (!grown) { + aether_fatal_flush(); /* the program's lines first (#3435, aether_fatal.h) */ + fprintf(stderr, "aether: out of memory recording an HTTP server thread\n"); /* silent-ok: last line before abort */ + abort(); + } + server->server_tids = grown; + server->server_tid_cap = cap; + } + server->server_tids[server->server_tid_count++] = aether_tid_self(); + pthread_mutex_unlock(&server->run_lock); +} + +void http_server_thread_leave(HttpServer* server) { + aether_tid_t self = aether_tid_self(); + pthread_mutex_lock(&server->run_lock); + for (int i = 0; i < server->server_tid_count; i++) { + if (aether_tid_equal(server->server_tids[i], self)) { + server->server_tids[i] = server->server_tids[--server->server_tid_count]; + break; + } + } + pthread_mutex_unlock(&server->run_lock); +} + +/* Whether the calling thread is one of the server's own; run_lock held. */ +static int server_thread_is_own_locked(HttpServer* server) { + aether_tid_t self = aether_tid_self(); + for (int i = 0; i < server->server_tid_count; i++) { + if (aether_tid_equal(server->server_tids[i], self)) return 1; + } + return 0; +} + +/* Begin a run, from either start: refused while one is on, so a second start + * cannot run a server twice over the same socket, pool and lists. */ +static int server_run_begin(HttpServer* server) { + pthread_mutex_lock(&server->run_lock); + if (server->serving) { + pthread_mutex_unlock(&server->run_lock); + return server_fail(server, "the server is already running"); + } + server->serving = 1; server->error[0] = '\0'; server->is_running = 1; atomic_store(&server->stopping, 0); + pthread_mutex_unlock(&server->run_lock); + return 0; +} + +/* The run is over: a stop waiting for it may return, and the server may be + * freed from here on, so the calling thread touches it no more after this. */ +static void server_run_end(HttpServer* server) { + pthread_mutex_lock(&server->run_lock); + server->serving = 0; + pthread_cond_broadcast(&server->run_done); + pthread_mutex_unlock(&server->run_lock); +} + +int http_server_start_raw(HttpServer* server) { + if (!server) return -1; + if (server_run_begin(server) < 0) return -1; return http_server_run(server); } -/* The server's body, with is_running already set by whoever started it: - * http_server_start_raw on the calling thread, or +/* The server's body, on the thread that runs it, with the run already begun + * by whoever started it: http_server_start_raw on the calling thread, or * http_server_start_background_raw before it spawns the thread. Never here, * on the server's own thread: a stop issued between the spawn and this * thread's first line was then overwritten by its `is_running = 1`, the loop - * ran forever, and http_server_stop, which joins the thread (#2672), hung. */ + * ran forever, and http_server_stop, which waits for the run (#2672), hung. + * A stop that came first has set stopping, and wins. */ static int http_server_run(HttpServer* server) { - int rc = http_server_serve(server); - monitor_stopped(server); + int rc = 0; + http_server_thread_enter(server); + if (!atomic_load(&server->stopping)) { + rc = http_server_serve(server); + monitor_stopped(server); + } + http_server_thread_leave(server); + server_run_end(server); return rc; } @@ -7813,10 +7893,22 @@ static int http_server_serve(HttpServer* server) { return 0; } +#if AETHER_HAS_THREADS +/* Join the background thread of a run that is over, if one is left: taken + * under run_lock, so of two callers one joins it, and joined outside it, + * since the thread ends its run under it. */ +static void server_join_background(HttpServer* server) { + pthread_mutex_lock(&server->run_lock); + int joinable = server->background_joinable; + pthread_t thread = server->background_thread; + server->background_joinable = 0; + pthread_mutex_unlock(&server->run_lock); + if (joinable) pthread_join(thread, NULL); +} +#endif + static void* http_server_background_main(void* arg) { HttpServer* server = (HttpServer*)arg; - server->background_tid = aether_tid_self(); - server->background_tid_set = 1; #if !defined(_WIN32) /* Embedded/background server: block async signals on this thread and * every thread it spawns (the accept thread + pool workers inherit @@ -7839,9 +7931,7 @@ static void* http_server_background_main(void* arg) { sigdelset(&block, SIGILL); pthread_sigmask(SIG_BLOCK, &block, NULL); #endif - /* is_running was set before this thread was spawned; a stop that came - * first has already cleared it and set stopping, and must win. */ - if (!atomic_load(&server->stopping)) http_server_run(server); + http_server_run(server); return NULL; } @@ -7856,21 +7946,26 @@ int http_server_start_background_raw(HttpServer* server) { (void)http_server_background_main; return -1; #else - /* Mark embedded mode before the thread starts so http_server_start_raw - * suppresses the interactive "Press Ctrl+C to stop" banner. */ - server->background = 1; - server->background_tid_set = 0; /* Running from here, not from the thread's first line: a stop issued as * soon as this returns must find a running server to stop, and nothing * the thread does later may undo it (see http_server_run). */ - server->is_running = 1; - atomic_store(&server->stopping, 0); - if (pthread_create(&server->background_thread, NULL, http_server_background_main, server) != 0) { + if (server_run_begin(server) < 0) return -1; + /* A thread left from an earlier run whose stop came from one of its own + * threads (a handler), which could not join it; that run is over. */ + server_join_background(server); + /* Embedded mode, set before the thread starts: no interactive banner. */ + server->background = 1; + pthread_mutex_lock(&server->run_lock); + int created = pthread_create(&server->background_thread, NULL, + http_server_background_main, server) == 0; + server->background_joinable = created; /* joined by http_server_stop */ + pthread_mutex_unlock(&server->run_lock); + if (!created) { server->background = 0; server->is_running = 0; - return -1; + server_run_end(server); + return server_fail(server, "cannot start the server thread"); } - server->background_joinable = 1; /* joined by http_server_stop */ return 0; #endif } @@ -7937,17 +8032,23 @@ void http_server_stop(HttpServer* server) { } pthread_mutex_unlock(&server->live_lock); -#if AETHER_HAS_THREADS - /* A background server is stopped when its thread is done: the closed - * socket ends its accept loop (at most its one-second poll on POSIX), - * and the loop shuts its pool down before it returns. Not from the - * thread itself, as when a handler stops the server it runs in. */ - if (server->background_joinable && - !(server->background_tid_set && - aether_tid_equal(aether_tid_self(), server->background_tid))) { - server->background_joinable = 0; - pthread_join(server->background_thread, NULL); + /* Wait for the run to be over, whichever start began it and on whatever + * thread: the closed socket and the wake pipe end its accept loop, and + * the loop joins its workers, which finish the request each is in, before + * it returns. Not on one of the run's own threads (a handler, the + * monitor), which the run waits for in turn: there the stop only + * signals, and the run ends once this call has returned. */ + pthread_mutex_lock(&server->run_lock); + int own = server_thread_is_own_locked(server); + if (!own) { + while (server->serving) pthread_cond_wait(&server->run_done, &server->run_lock); } + pthread_mutex_unlock(&server->run_lock); + +#if AETHER_HAS_THREADS + /* A background run's thread, once its run is over: it does nothing + * after ending it, so this is quick. */ + if (!own) server_join_background(server); #endif } @@ -7955,6 +8056,18 @@ void http_server_free(HttpServer* server) { if (!server) return; http_server_stop(server); + /* Still running only when this is one of the run's own threads, where + * the stop could not wait: the run is still using the server. */ + pthread_mutex_lock(&server->run_lock); + int running = server->serving; + pthread_mutex_unlock(&server->run_lock); + if (running) { + aether_fatal_flush(); /* the program's lines first (#3435, aether_fatal.h) */ + fprintf(stderr, "aether: http_server_free called on one of the server's own threads " /* silent-ok: last line before abort */ + "(a handler or its monitor) while it runs; stop it there and free it " + "from the thread that started it\n"); + abort(); + } http_server_monitor_null_raw(server); /* gives back the closures' environments */ @@ -8037,6 +8150,9 @@ void http_server_free(HttpServer* server) { #endif free(server->live_fds); + free(server->server_tids); + pthread_cond_destroy(&server->run_done); + pthread_mutex_destroy(&server->run_lock); pthread_mutex_destroy(&server->live_lock); pthread_mutex_destroy(&server->tls_lock); pthread_mutex_destroy(&server->tls_config_lock); diff --git a/std/net/aether_http_server.h b/std/net/aether_http_server.h index 06c01953b..606a6b476 100644 --- a/std/net/aether_http_server.h +++ b/std/net/aether_http_server.h @@ -349,16 +349,30 @@ typedef struct HttpServer { // intercept a process-directed signal meant for the host application. // std-http-server-background-sigurg-poisons-harness.md int background; - // The background server's thread, which http_server_stop joins so the - // accept loop and its workers are done before stop returns: left - // detached, they were still in Winsock calls when the program went on - // to exit (an access violation on Windows). background_tid is set by - // the thread itself, so a stop called from a handler does not join - // the thread it runs on. + // The background server's thread, which http_server_stop joins once the + // run is over (see `serving`): left detached, it was still in Winsock + // calls when the program went on to exit (an access violation on + // Windows). pthread_t background_thread; int background_joinable; - int background_tid_set; - aether_tid_t background_tid; + // The run, whichever start began it. `serving` is set by the start, + // under run_lock, and cleared once the run is over: its accept loop has + // returned and its workers, event loop and parking lot are joined and + // freed; run_done is broadcast then. http_server_stop waits for it, so + // when stop returns nothing of the server is running on any thread and + // http_server_free may free it. A foreground start on another thread (an + // actor's) used to be signalled but not waited for, and the free that + // followed released the server under a worker still serving a keep-alive + // connection. server_tids are the threads the run owns (the one running + // it, its pool workers, the event loop's drivers): a stop called on one + // of them, from a handler or the monitor, cannot wait for a run that + // waits for it, so it only signals, and a free there is refused. + pthread_mutex_t run_lock; + pthread_cond_t run_done; + int serving; + aether_tid_t* server_tids; + int server_tid_count; + int server_tid_cap; // Multi-accept: one accept thread per core with SO_REUSEPORT (opt-in) int multi_accept; // 0 = single accept (default), 1 = SO_REUSEPORT multi-accept int accept_thread_count; @@ -434,10 +448,26 @@ int http_server_has_monitor(HttpServer* server); // that does not resolve fails the bind (#2639). No-op if `host` is NULL // or empty. void http_server_set_host(HttpServer* server, const char* host); +// Start the server on the calling thread (returns when it stops) or on a +// thread of its own (returns at once). Either refuses, -1 with the reason in +// http_server_error, a server that is already running. int http_server_start_raw(HttpServer* server); int http_server_start_background_raw(HttpServer* server); +// Stop the server and wait until its run is over, whichever start began it +// and whichever thread it runs on: when stop returns, its accept loop and +// every worker are done, and the server may be freed or started again. +// Called on one of the server's own threads (a handler, the monitor), it +// only signals: the run ends once that call returns. void http_server_stop(HttpServer* server); +// Stop (as above) and free the server. Fatal on one of the server's own +// threads while it runs, since the run still uses it: stop it there and +// free it from the thread that started it. void http_server_free(HttpServer* server); +// The run's threads say they are the server's, for the duration of the +// run, so a stop on them does not wait for itself. Internal to std.net: +// the pool workers and the event loop's drivers call them. +void http_server_thread_enter(HttpServer* server); +void http_server_thread_leave(HttpServer* server); // Enable HTTP/1.1 keep-alive on this server (#260 Tier 0). When // `enabled` is non-zero, the connection handler reads multiple diff --git a/tests/ae_sweep_prune.txt b/tests/ae_sweep_prune.txt index fd4186d66..3ca208bff 100644 --- a/tests/ae_sweep_prune.txt +++ b/tests/ae_sweep_prune.txt @@ -175,6 +175,7 @@ tests/integration/http_serve_and_dial/ tests/integration/http_server_actor_dispatch/ tests/integration/http_server_background_quiet/ tests/integration/http_server_monitor/ +tests/integration/http_server_stop_waits/ tests/integration/http_server_multi_accept/ tests/integration/http_server_connect_tunnel/ tests/integration/http_server_h2_middleware/ diff --git a/tests/integration/http_server_monitor/test_http_server_monitor.sh b/tests/integration/http_server_monitor/test_http_server_monitor.sh index bad27809a..0d3f2dc88 100755 --- a/tests/integration/http_server_monitor/test_http_server_monitor.sh +++ b/tests/integration/http_server_monitor/test_http_server_monitor.sh @@ -11,7 +11,16 @@ AE="$ROOT/build/ae" fail() { echo " [FAIL] http_server_monitor: $1"; exit 1; } -run() { MONITOR_MODE="$1" AETHER_HOME="$ROOT" "$AE" run "$SCRIPT_DIR/$2" 2>&1; } +# Build, then run: the build's output (a linker warning about a Homebrew dylib, +# say) is not the program's, and only the program's is under test. A build +# that fails shows its own output. +BUILD_DIR="$(mktemp -d)"; trap 'rm -rf "$BUILD_DIR"' EXIT +run() { + _exe="$BUILD_DIR/$(basename "$2" .ae)" + AETHER_HOME="$ROOT" "$AE" build "$SCRIPT_DIR/$2" -o "$_exe" >"$BUILD_DIR/build.log" 2>&1 \ + || { cat "$BUILD_DIR/build.log"; return 1; } + MONITOR_MODE="$1" "$_exe" 2>&1 +} OUT="$(run - monitor.ae)" || { echo "$OUT"; fail "the monitor program failed"; } [ "$OUT" = "MONITOR-OK" ] || { echo "$OUT"; fail "a background server or a failed bind printed something"; } @@ -34,7 +43,7 @@ FG-DONE" case "$(uname -s 2>/dev/null)" in MINGW*|MSYS*|CYGWIN*|Windows_NT) ;; *) - TMPDIR="$(mktemp -d)"; trap 'rm -rf "$TMPDIR"' EXIT + TMPDIR="$BUILD_DIR" AETHER_HOME="$ROOT" "$AE" build "$SCRIPT_DIR/actor_mode.ae" -o "$TMPDIR/actor_mode" \ --extra "$SCRIPT_DIR/actor_shim.c" >"$TMPDIR/build.log" 2>&1 \ || { cat "$TMPDIR/build.log"; fail "the actor-mode program did not build"; } diff --git a/tests/integration/http_server_stop_waits/foreground_actor.ae b/tests/integration/http_server_stop_waits/foreground_actor.ae new file mode 100644 index 000000000..d6f973dd1 --- /dev/null +++ b/tests/integration/http_server_stop_waits/foreground_actor.ae @@ -0,0 +1,120 @@ +// http_server_stop_waits: a foreground server (http.server_start, so +// http_server_start_raw) run by an actor, so on another thread, with a +// keep-alive connection held open across the stop, then freed at once, round +// after round. +// +// http_server_stop used to wait only for a server started in the +// background. For a foreground one on another thread it signalled and +// returned, and the free that followed released the server while its accept +// loop and a pool worker, woken from waiting on the held connection, were +// still in it: a segfault in the worker (conn_recv_request_start, +// http_resolve_route) on macOS and on Linux. Now stop waits for the run, +// whichever start began it, so the server's monitor has reported stopped by +// the time stop returns, which each round checks, and the free is safe. + +import std.http +import std.os +import std.fs +import std.string +import std.tcp + +extern http_server_stop(server: ptr) +extern http_server_free(server: ptr) +extern http_server_port(server: ptr) -> int +extern sleep(ms: int) +extern exit(code: int) + +message Run { srv: ptr } +actor Runner { + state runs = 0 + receive { + Run(srv) -> { + runs = runs + 1 + http.server_start(srv) + } + } +} + +handler(req: ptr, res: ptr, ud: ptr) { + http.response_set_status(res, 200) + http.response_set_body(res, "ok") +} + +// One request on a connection the caller keeps: the server holds it open +// for the next request, so a pool worker waits on it. "" when the answer +// came back, else what went wrong. +ask(sock: ptr) -> string { + tcp.write(sock, "GET /ok HTTP/1.1\r\nHost: 127.0.0.1\r\n\r\n") + wire = "" + waited = 0 + while string.index_of(wire, "\r\n\r\nok") < 0 && waited < 50 { + ready, _perr = tcp.poll(sock, 100) + if ready { + chunk, rerr = tcp.read(sock, 4096) + if rerr != "" || string.length(chunk) == 0 { + return "connection closed before the answer" + } + wire = string.concat(wire, chunk) + } + waited = waited + 1 + } + if string.index_of(wire, "HTTP/1.1 200") != 0 { + return "no 200 answer" + } + return "" +} + +main() { + dir = os.getenv("STOP_WAITS_DIR") + if dir == "" { + dir = "." + } + rounds = 10 + i = 0 + while i < rounds { + marker = "${dir}/stopped-${i}" + fs.delete(marker) + s = http.server_create(0) + http.server_set_host(s, "127.0.0.1") + http.server_get(s, "/ok", handler, null) + http.monitor(s) { + listening() | host: string, port: int | {} + stopped() | host: string, port: int | { + fs.write(marker, "stopped") + } + } + if http.server_bind(s, "127.0.0.1", 0) != "" { + println("FAIL round ${i}: bind") + exit(1) + } + port = http_server_port(s) + runner = spawn(Runner()) + runner ! Run { srv: s } + + // The actor may not have started the server yet: the socket is + // bound, so the connect is queued and answered once it has. + sock, cerr = tcp.connect("127.0.0.1", port) + if cerr != "" { + println("FAIL round ${i}: connect: ${cerr}") + exit(1) + } + why = ask(sock) + if why != "" { + println("FAIL round ${i}: ${why}") + exit(1) + } + + // The connection stays open across the stop and the free. + http_server_stop(s) + if fs.exists(marker) == 0 { + println("FAIL round ${i}: http_server_stop returned before the server stopped") + exit(1) + } + http_server_free(s) + sleep(50) + tcp.close(sock) + fs.delete(marker) + i = i + 1 + } + println("FOREGROUND-OK ${rounds}") +} diff --git a/tests/integration/http_server_stop_waits/from_handler.ae b/tests/integration/http_server_stop_waits/from_handler.ae new file mode 100644 index 000000000..e0629c811 --- /dev/null +++ b/tests/integration/http_server_stop_waits/from_handler.ae @@ -0,0 +1,97 @@ +// http_server_stop_waits: a stop from a handler, which runs on one of the +// server's own pool workers, and the start that is refused while a server +// runs. +// +// A stop waits for the server's run to be over, but the run joins its +// workers, so on one of them it only signals; the thread that started the +// server, or any other, then stops (waiting) and frees it. A background +// server stopped from a handler used to hang: the stop joined the server +// thread, which was joining the worker the stop ran on. + +import std.http +import std.string + +extern http_server_port(server: ptr) -> int +extern http_server_free(server: ptr) +extern exit(code: int) + +message Run { srv: ptr } +actor Runner { + state runs = 0 + receive { + Run(srv) -> { + runs = runs + 1 + http.server_start(srv) + } + } +} + +stop_handler(req: ptr, res: ptr, ud: ptr) { + http.response_set_status(res, 200) + http.response_set_body(res, "stopping") + http.server_stop(ud) +} + +// A server with /stop, on 127.0.0.1 and a port the kernel picks, bound. +make_server() -> ptr { + s = http.server_create(0) + http.server_set_host(s, "127.0.0.1") + http.null_monitor(s) + http.server_get(s, "/stop", stop_handler, s) + if http.server_bind(s, "127.0.0.1", 0) != "" { + println("FAIL: bind") + exit(1) + } + return s +} + +ask_stop(s: ptr, what: string) { + port = http_server_port(s) + body, err = http.get("http://127.0.0.1:${port}/stop") + if err != "" || body != "stopping" { + println("FAIL ${what}: /stop answered '${body}' (${err})") + exit(1) + } +} + +main() { + i = 0 + while i < 5 { + // Foreground, on an actor. + s = make_server() + runner = spawn(Runner()) + runner ! Run { srv: s } + ask_stop(s, "foreground") + http_server_free(s) + + // Background. + b = make_server() + if http.server_start_background(b) != "" { + println("FAIL: start_background") + exit(1) + } + ask_stop(b, "background") + http_server_free(b) + i = i + 1 + } + + // A start while the server runs is refused, whichever start it is. + r = make_server() + if http.server_start_background(r) != "" { + println("FAIL: start_background") + exit(1) + } + again = http.server_start_background(r) + if string.index_of(again, "already running") < 0 { + println("FAIL: a second background start was not refused: '${again}'") + exit(1) + } + fg = http.server_start(r) + if string.index_of(fg, "already running") < 0 { + println("FAIL: a foreground start of a running server was not refused: '${fg}'") + exit(1) + } + ask_stop(r, "refused-start server") + http_server_free(r) + println("FROM-HANDLER-OK") +} diff --git a/tests/integration/http_server_stop_waits/test_http_server_stop_waits.sh b/tests/integration/http_server_stop_waits/test_http_server_stop_waits.sh new file mode 100755 index 000000000..450cc5b12 --- /dev/null +++ b/tests/integration/http_server_stop_waits/test_http_server_stop_waits.sh @@ -0,0 +1,67 @@ +#!/bin/sh +# std.http: http_server_stop waits for the server's run, whichever start +# began it, so http_server_free after it is safe. +# +# foreground_actor.ae: http_server_start_raw run by an actor, a keep-alive +# connection held open across the stop, stop and free from main. Stop used +# to wait only for a background server, and the free released a foreground +# one under its pool worker: a segfault on macOS and Linux (found by +# servirtium-vcr). Each round also checks the server's monitor reported +# stopped before stop returned. +# +# from_handler.ae: a stop from a handler (one of the server's own threads) +# only signals, and the free from main then waits; a background server +# stopped that way used to hang. A start of a running server is refused. +# +# A watchdog kills each program after LIMIT seconds so a hang fails rather +# than wedging the sweep. +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +ROOT="$(cd "$SCRIPT_DIR/../../.." && pwd)" +NAME=http_server_stop_waits +EXE="${EXE_EXT:-}" +if [ -z "$EXE" ] && [ ! -x "$ROOT/build/ae" ] && [ -x "$ROOT/build/ae.exe" ]; then + EXE=".exe" +fi +AE="$ROOT/build/ae$EXE" +[ -x "$AE" ] || { echo " [SKIP] $NAME: build/ae not built"; exit 0; } +LIMIT="${STOP_WAITS_LIMIT:-60}" +. "$ROOT/tests/lib/server_jobs.sh" + +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP" || true' EXIT + +run_case() { + prog="$1" + want="$2" + if ! "$AE" build "$SCRIPT_DIR/$prog.ae" -o "$TMP/$prog$EXE" > "$TMP/$prog.build.log" 2>&1; then + echo " [FAIL] $NAME: $prog.ae did not build" + tail -8 "$TMP/$prog.build.log" | sed 's/^/ /' + return 1 + fi + STOP_WAITS_DIR="$TMP" "$TMP/$prog$EXE" > "$TMP/$prog.log" 2>&1 & + pid=$! + waited=0 + while server_alive "$pid"; do + if [ "$waited" -ge "$LIMIT" ]; then + kill_server "$pid" + echo " [FAIL] $NAME: $prog did not finish in ${LIMIT}s (a stop hung)" + tail -5 "$TMP/$prog.log" | sed 's/^/ /' + return 1 + fi + sleep 1 + waited=$((waited + 1)) + done + wait "$pid" + rc=$? + if [ "$rc" -ne 0 ] || ! grep -q "$want" "$TMP/$prog.log"; then + echo " [FAIL] $NAME: $prog exited $rc" + tail -5 "$TMP/$prog.log" | sed 's/^/ /' + return 1 + fi + return 0 +} + +run_case foreground_actor "FOREGROUND-OK 10" || exit 1 +run_case from_handler "FROM-HANDLER-OK" || exit 1 +echo " [PASS] $NAME: a foreground server stopped and freed from another thread, a stop from a handler, a refused second start" +exit 0 diff --git a/tests/regression/test_bytes_string_from_ptr_handle.ae b/tests/regression/test_bytes_string_from_ptr_handle.ae new file mode 100644 index 000000000..a3ace575b --- /dev/null +++ b/tests/regression/test_bytes_string_from_ptr_handle.ae @@ -0,0 +1,50 @@ +// Regression: bytes.string_from_ptr wants a raw data pointer, but a bytes +// handle is also a `ptr`, so passing the handle silently copied the +// AetherBytes struct header as text (servirtium-vcr's Azure Key Vault test +// got garbage keys). A handle is now refused with a panic; the correct +// forms (to_string on the handle, string_from_ptr on bytes.data) still work, +// binary-safe (NUL and 0xff bytes). + +import std.bytes +import std.string + +extern exit(code: int) + +fn check(cond: bool, msg: string) { + if !cond { + println("FAIL: ${msg}") + exit(1) + } +} + +main() { + b = bytes.new(8) + _ = bytes.set(b, 0, 65) + _ = bytes.set(b, 1, 0) + _ = bytes.set(b, 2, 255) + _ = bytes.set(b, 3, 66) + _ = bytes.set_length(b, 4) + + s1 = bytes.to_string(b, 4) + check(string.length(s1) == 4, "to_string length") + check(string.char_at(s1, 0) == 65 && string.char_at(s1, 1) == 0, "to_string bytes 0,1") + check(string.char_at(s1, 2) == 255 && string.char_at(s1, 3) == 66, "to_string bytes 2,3") + + s2 = bytes.string_from_ptr(bytes.data(b), 4) + check(string.length(s2) == 4, "string_from_ptr(data) length") + check(string.char_at(s2, 0) == 65 && string.char_at(s2, 1) == 0, "string_from_ptr(data) bytes 0,1") + check(string.char_at(s2, 2) == 255 && string.char_at(s2, 3) == 66, "string_from_ptr(data) bytes 2,3") + + // The trap: the handle itself is refused, not copied as header bytes. + refused = 0 + try { + bad = bytes.string_from_ptr(b, 4) + println("FAIL: handle accepted, length ${string.length(bad)}") + } catch e { + if string.contains(e, "bytes.to_string") == 1 { refused = 1 } + } + check(refused == 1, "string_from_ptr(handle) must panic naming bytes.to_string") + + bytes.free(b) + println("PASS: string_from_ptr refuses a bytes handle; data/raw forms work") +} diff --git a/tools/ae.c b/tools/ae.c index bd635d4db..35b1f0d36 100644 --- a/tools/ae.c +++ b/tools/ae.c @@ -57,6 +57,7 @@ extern char** environ; #ifdef __APPLE__ #include +#include #endif #if defined(__FreeBSD__) || defined(__NetBSD__) #include @@ -5060,6 +5061,34 @@ static const char* ae_vendored_src_defs(void) { return defs; } +#ifdef __APPLE__ +/* A program built here runs here. With MACOSX_DEPLOYMENT_TARGET unset, Apple's + * toolchain targets the SDK's macOS, which a Command Line Tools or Xcode + * install often leaves behind the running OS; Homebrew builds its dylibs for + * the running OS, and ld then warns on every link ("building for macOS-27.0, + * but linking with dylib ... built for newer version 27.2"). The warning goes + * to the build's stderr, so it lands in the output of `ae run`. Default the + * target to the running OS's major.minor; a value the caller set wins, so a + * build that deliberately targets an older macOS keeps doing so. Native + * builds only: the zig cross path names its own target. */ +static bool running_macos_target(char* out, size_t size) { + char ver[64]; + size_t len = sizeof(ver); + if (sysctlbyname("kern.osproductversion", ver, &len, NULL, 0) != 0) return false; + int major = 0, minor = 0; + if (sscanf(ver, "%d.%d", &major, &minor) < 1 || major < 11) return false; + snprintf(out, size, "%d.%d", major, minor); + return true; +} + +static void default_macos_deployment_target(void) { + if (getenv("MACOSX_DEPLOYMENT_TARGET")) return; + char target[32]; + if (running_macos_target(target, sizeof(target))) + setenv("MACOSX_DEPLOYMENT_TARGET", target, 0); +} +#endif + void build_gcc_cmd(char* cmd, size_t size, const char* c_file, const char* out_file, bool optimize, const char* extra_files) { @@ -5365,6 +5394,9 @@ void build_gcc_cmd(char* cmd, size_t size, return; } } +#ifdef __APPLE__ + default_macos_deployment_target(); +#endif /* contrib.sqlite with no archive here: compile it (see * vendored_sqlite_link). */ ae_link = vendored_sqlite_link(cc, ae_link, user_cflags); @@ -13137,6 +13169,18 @@ static int cmd_cflags(int argc, char** argv) { if (wrote_anything) fputc(' ', stdout); fputs("-pthread -lm", stdout); wrote_anything = 1; +#ifdef __APPLE__ + /* The same default `ae build` applies (default_macos_deployment_target), + * for the caller's own compiler, which `ae` cannot set an environment + * for: without it ld warns when a Homebrew dylib named below was built + * for a newer macOS than the SDK's. Omitted when the caller set + * MACOSX_DEPLOYMENT_TARGET, which the compiler reads itself. */ + if (!getenv("MACOSX_DEPLOYMENT_TARGET")) { + char target[32]; + if (running_macos_target(target, sizeof(target))) + printf(" -mmacosx-version-min=%s", target); + } +#endif /* Windows system libraries. libaether references Winsock, the Crypto * API and dbghelp's symboliser regardless of which std modules the