Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions docs/limitations.md
Original file line number Diff line number Diff line change
Expand Up @@ -223,3 +223,40 @@ payload (the caller-side cascade does not yet treat a call argument as the
borrow it now provably is), and extracting the same optional local twice is a
rare use-after-free that needs a second-extraction check rather than the blanket
retain that was tried and reverted for regressing the common single case.

sweeping std's shared globals for the same class of bug turned up three things
that are questions of design rather than repairs, so they are recorded here
instead of decided.

`std.metrics` is correct under concurrency and does not scale. one mutex covers
all thirteen registries, so every counter increment, gauge set and histogram
observation in the process serializes on it, and a metric written once per
request is the normal case. measured on the 2-core box, one task manages 7.4M
increments a second; two manage 3.7M between them, four 3.0M, eight 2.4M — the
aggregate falls as tasks are added, which is what a single lock looks like. the
absolute cost is still small next to a request, around 0.4 µs per increment at
eight tasks, so nothing is on fire. it is the shape that will not hold if a
process ever writes metrics faster than it serves requests. every way out costs
something. sharding by metric name spreads the contention but makes a coherent
snapshot harder to take. a lock per series adds a lookup to the hot path.
atomic counters are the obvious answer for a counter and no answer at all for a
histogram, which updates seven values as one unit.

`std.net.tls` configs are never closed, and `std.net.http` builds one per https
request. `Config.close()` exists and removes the config's entries from the
registry, but nothing in std or the examples calls it, so every https request
adds another copy of the system root bundle — 219 KB here — to a map that only
grows. two hundred configs held 44 MB that closing them released. the fix is an
ownership decision rather than a patch: either the http client closes the config
it built, which means being sure no response still refers to it, or the default
client config becomes a process-wide value built once, which changes what
`tls.client_config()` returns.

`std.args` is safe by convention, with nothing enforcing the convention. its ten
globals have no lock, which is fine given the parser's shape: init, then the
add_flag / add_option / add_positional calls, then parse, all from main, after
which everything left only reads. no path in the module mutates from the query
side, so the state really is fixed once parse returns. nothing stops a program
from calling the setup half from a spawned task, though, and there would be no
diagnostic if it did — just a torn string. a lock here is cheap. whether a cli
argument parser should carry one is the part worth an opinion.
8 changes: 8 additions & 0 deletions std/args.pith
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@
# cross-module access in the current runtime
# ===============================================================

# these are shared globals with no lock, and that is safe because the parser has
# exactly one shape: init, add_flag / add_option / add_positional, then parse,
# all from main before the program does any work. every function that writes is
# one of those setup calls, and every function that reads (get_flag, get_option,
# get_positional, has_errors, get_errors, help_text) only reads — there is no
# path in the module that mutates from the query side, so once parse returns the
# state is fixed for the life of the process. calling the setup half from a
# spawned task is not supported and would need a lock.
mut app_name: String := ""
mut app_desc: String := ""
mut flags_data: String := ""
Expand Down
10 changes: 9 additions & 1 deletion std/db/mysql.pith
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ import std.time as time
# for two reasons: a Db then carries only a plain handle, which sidesteps a
# cross-module retain bug when a caller holds the handle; and one lock covers
# every pool, so concurrent tasks borrow and return connections safely.
# every read of `pools` — open, the three acquire/release/discard helpers, and
# Db.close — takes registry_mu around the lookup and nothing else, so a slow
# connect or close happens off the lock.
mut pools: List[mysql.Pool] := []
mut registry_mu := Mutex()

Expand Down Expand Up @@ -117,12 +120,17 @@ struct Pin:
mut broken: Bool
mut finished: Bool

# every access to `pins` is one of the small helpers below, and each takes
# pins_mu around exactly one field read or write; the connection i/o that
# follows (release, discard) happens after the unlock.
mut pins: List[Pin] := []
mut pins_mu := Mutex()

# the server-side prepared statement behind each Stmt, indexed by the slot the
# Stmt carries. it lives beside the pin registry (not inside Pin) so a Tx, which
# prepares nothing lasting, needs no placeholder statement.
# prepares nothing lasting, needs no placeholder statement — and it is guarded
# by pins_mu, the same lock, since register_prepared and prepared_stmt are its
# only two accesses.
mut prepared: List[mysql.Stmt] := []

# pin a fresh connection from pool `pool_handle` and register it, returning the
Expand Down
6 changes: 6 additions & 0 deletions std/db/postgres.pith
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ import std.time as time
# for two reasons: a Db then carries only a plain handle, which sidesteps a
# cross-module retain bug when a caller holds the handle; and one lock covers
# every pool, so concurrent tasks borrow and return connections safely.
# every read of `pools` — open, the three acquire/release/discard helpers, and
# Db.close — takes registry_mu around the lookup and nothing else, so a slow
# connect or close happens off the lock.
mut pools: List[postgres.Pool] := []
mut registry_mu := Mutex()

Expand Down Expand Up @@ -117,6 +120,9 @@ struct Pin:
mut broken: Bool
mut finished: Bool

# every access to `pins` is one of the small helpers below, and each takes
# pins_mu around exactly one field read or write; the connection i/o that
# follows (release, discard) happens after the unlock.
mut pins: List[Pin] := []
mut pins_mu := Mutex()

Expand Down
29 changes: 27 additions & 2 deletions std/log.pith
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ pub struct Event:
pub message: String
pub fields: List[Field]

# the global logger's settings, and the lock over them. reconfiguration is not
# a startup-only step: set_level, set_format and set_file are public, documented
# as plain calls with no ordering requirement, and a server that raises its log
# level while it is serving is the obvious reason to have them. so these are
# written from one task while every other task is reading them on its way
# through root() — and sink_path is a string, whose reassignment releases the
# old value under a reader that has just loaded it. the lock covers the reads
# and the writes; it is never held across the log write itself, which happens
# from emit_record on a Logger that already carries its own copy.
mut settings_mu := Mutex()
mut min_level: Int := LEVEL_DEBUG
mut sink_path: String := ""
mut output_format: Int := FORMAT_CONSOLE
Expand Down Expand Up @@ -141,9 +151,16 @@ fn add_field(base: List[Field], item: Field) -> List[Field]:
out.push(item)
return out

# create a logger from the current global settings.
# create a logger from the current global settings. the three are read together
# so a logger never mixes a level from before a reconfiguration with a sink from
# after it.
pub fn root() -> Logger:
return Logger(min_level, output_format, sink_path, [], "", "", "")
settings_mu.lock()
level := min_level
format := output_format
path := sink_path
settings_mu.unlock()
return Logger(level, format, path, [], "", "", "")

# create a compact trace id suitable for logs and future propagation.
pub fn new_trace_id() -> String!:
Expand All @@ -161,11 +178,15 @@ pub fn set_level(level: Int):
log_set_level(level)

fn log_set_level(level: Int):
settings_mu.lock()
min_level = level
settings_mu.unlock()

# set the global output format.
pub fn set_format(format: Int):
settings_mu.lock()
output_format = format
settings_mu.unlock()

# use console output for the global logger.
pub fn set_console():
Expand All @@ -180,14 +201,18 @@ pub fn set_file(path: String):
log_set_file(path)

fn log_set_file(path: String):
settings_mu.lock()
sink_path = path
settings_mu.unlock()

# stop writing log output to a file sink.
pub fn clear_file():
log_clear_file()

fn log_clear_file():
settings_mu.lock()
sink_path = ""
settings_mu.unlock()

impl Logger:
fn with(fields: List[Field]) -> Logger:
Expand Down
7 changes: 7 additions & 0 deletions std/metrics.pith
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ mut histogram_bucket_defs: Map[String, List[Float]] := {}
# guards all the metric maps so concurrent updates from separate threads do not
# race. each public operation takes it around its whole read-modify-write; the
# internal helpers (ensure_*, *_value, render_*) assume it is already held.
#
# every access to every map in this module goes through that rule — checked, not
# assumed: nothing reads a map outside the lock, which is the failure a
# write-guarded registry usually still has. holding it across a whole
# snapshot_text() render is deliberate and safe: rendering is string building
# with no i/o in it, so it cannot park a green task, and the scrape's socket
# write happens in std.prometheus after this module has handed the text back.
mut metrics_mu := Mutex()

# label support. a metric can have several series, one per distinct label set
Expand Down
13 changes: 13 additions & 0 deletions std/net/grpc.pith
Original file line number Diff line number Diff line change
Expand Up @@ -1202,6 +1202,15 @@ EXPIRED_ON_ARRIVAL := "grpc: deadline expired before the handler ran"
fn expired_response() -> http.HttpResponse:
return http.response(200).content_type(GRPC_CONTENT_TYPE).with_trailers(grpc_status_trailers(GRPC_DEADLINE_EXCEEDED, EXPIRED_ON_ARRIVAL))

# the process-wide unary dispatch, and the same for the body-level and streaming
# forms further down. all three are shared and unlocked, and safe because of
# where they are written: each is assigned as the first statement of the serve()
# call that installs it, and that call then blocks in the accept loop for the
# life of the process. so the write happens before the listener exists, let
# alone the first request task, and nothing writes them again. the cost is one
# dispatch per process — two concurrent serve() calls would fight over the slot,
# which is the same single-handler constraint std.net.http2.server documents for
# its streaming handler.
mut grpc_dispatch_fn: fn(String, Bytes) -> Bytes!GrpcError := grpc_no_dispatch

fn grpc_no_dispatch(path: String, request: Bytes) -> Bytes!GrpcError:
Expand Down Expand Up @@ -1281,6 +1290,8 @@ pub fn serve_tls(host: String, port: Int, cert: String, key: String, dispatch: f
# this, deframing per method. buffered means the whole request and reply pass
# through memory — the right tool for bounded streams, not endless ones.

# installed by serve_body / serve_body_tls; see grpc_dispatch_fn above for why
# this needs no lock.
mut grpc_body_dispatch_fn: fn(String, Bytes) -> Bytes!GrpcError := grpc_no_dispatch

fn grpc_serve_body_handler(req: http.HttpRequestBytes) -> http.HttpResponse:
Expand Down Expand Up @@ -1457,6 +1468,8 @@ impl GrpcStream:
# the stream dispatch lives in a module global for the same reason the unary
# dispatch does: a pith closure cannot capture a fn-value, so there is one
# streaming grpc server per process.
# installed by the streaming serve calls; see grpc_dispatch_fn above for why
# this needs no lock.
mut grpc_stream_dispatch_fn: fn(String, GrpcStream) -> Int!GrpcError := grpc_no_stream_dispatch

fn grpc_no_stream_dispatch(path: String, call: GrpcStream) -> Int!GrpcError:
Expand Down
9 changes: 8 additions & 1 deletion std/net/http2/server.pith
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ MAX_CONNECTIONS := 512
# the in-flight permits, shared between the accept loops and the connection
# tasks. a Semaphore is safe to share across threads; it lives as a module
# global rather than a spawned-task argument (which the compiler cannot yet
# dispatch for a sync-primitive type).
# dispatch for a sync-primitive type). it is `mut` only because that is how a
# global holding a constructed value is declared — the binding itself is never
# reassigned, and the permit count is the semaphore's own synchronized state.
mut connection_slots := Semaphore(MAX_CONNECTIONS)

# how long a connection may sit without the socket making progress before the
Expand Down Expand Up @@ -554,6 +556,11 @@ impl ServerStream:
# fn(ServerStream) would make Server and ServerStream mutually recursive, which
# the type system resolves in file order and so cannot express. the cost is one
# streaming handler per process — the same shape grpc already documents.
#
# shared and unlocked, and safe for the same reason grpc's dispatch is: the
# assignment is the first statement of the listen_*_streaming call that installs
# it, and that call then blocks in the accept loop, so it lands before the
# listener exists and is never written again.
mut stream_handler_fn: fn(ServerStream) -> Int := stream_handler_unset

# the default streaming handler: refuse the stream, so a misconfigured server
Expand Down
Loading
Loading