config: harden the parallax registration call
protect_tail_call never matched, since correlate prefixes every line with a space, and Lua keeps the main chunk frame when tail-calling a C function anyway. Assisted-by: Claude Opus 5.5 via Claude Code Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
diff --git a/src/error.rs b/src/error.rs
index 4170f51..78cc34b 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -11,15 +11,12 @@ pub enum Error {
)]
Config(#[from] mlua::Error),
- #[error("{} must call ({what} ...)", .path)]
+ #[error("{path} must call (parallax ...)")]
#[diagnostic(
code(parallax::config_registration),
help("a config registers with (parallax {{...}})")
)]
- ConfigRegistration {
- path: Utf8PathBuf,
- what: &'static str,
- },
+ ConfigRegistration { path: Utf8PathBuf },
#[error("io error: {0}")]
#[diagnostic(code(parallax::io))]
diff --git a/src/parallax.rs b/src/parallax.rs
index 9e700b0..1982b25 100644
--- a/src/parallax.rs
+++ b/src/parallax.rs
@@ -1,7 +1,10 @@
use camino::Utf8Path;
use camino::Utf8PathBuf;
+use std::mem;
use std::sync::Arc;
use std::sync::Mutex;
+use std::sync::MutexGuard;
+use std::sync::PoisonError;
use mlua::ErrorContext as _;
use mlua::FromLua;
@@ -10,6 +13,7 @@ use mlua::Lua;
use mlua::LuaSerdeExt as _;
use mlua::Table;
use mlua::Value;
+use mlua::debug::Debug;
use crate::Error;
use crate::spec::Setup;
@@ -32,14 +36,14 @@ impl FromLua for Config {
let Value::Table(table) = value else {
return Err(error_at(lua, "parallax takes a table"));
};
- let server = pop_function(&table, "server", lua)?;
- let flow = pop_function(&table, "flow", lua)?;
+ // Popping from a copy leaves the config's own table intact.
+ let spec =
+ shallow_copy(lua, &table).map_err(|error| error_at(lua, &error_message(&error)))?;
+ let server = pop_function(&spec, "server", lua)?;
+ let flow = pop_function(&spec, "flow", lua)?;
let setup: Setup = lua
- .from_value(Value::Table(table))
+ .from_value(Value::Table(spec))
.map_err(|error| error_at(lua, &error_message(&error)))?;
- if setup.variants.is_empty() {
- return Err(error_at(lua, "variants must not be empty"));
- }
Ok(Config {
server,
flow,
@@ -52,21 +56,28 @@ impl FromLua for Config {
/// never sees it and unknown-field validation still catches typos.
fn pop_function(table: &Table, key: &str, lua: &Lua) -> Result<Function, mlua::Error> {
let call = table
- .get::<Value>(key)
+ .raw_get::<Value>(key)
.map_err(|error| error_at(lua, &error_message(&error)))?;
let Value::Function(call) = call else {
return Err(error_at(lua, &format!("{key} must be a function")));
};
table
- .set(key, Value::Nil)
+ .raw_set(key, Value::Nil)
.map_err(|error| error_at(lua, &error_message(&error)))?;
Ok(call)
}
+fn shallow_copy(lua: &Lua, table: &Table) -> Result<Table, mlua::Error> {
+ let copy = lua.create_table()?;
+ for pair in table.pairs::<Value, Value>() {
+ let (key, value) = pair?;
+ copy.raw_set(key, value)?;
+ }
+ Ok(copy)
+}
+
/// A loaded parallax: the executable form of a config. The server
-/// function and flow are owned handles into the embedded Lua state,
-/// which stays alive in this struct; fields are ordered so the handles
-/// drop while the VM lives.
+/// function and flow are handles into the embedded Lua state.
pub struct Parallax {
// Consumed once running variants lands.
#[allow(dead_code)]
@@ -78,6 +89,8 @@ pub struct Parallax {
name: String,
#[allow(dead_code)]
path: Utf8PathBuf,
+ // Never read, but the handles above only hold weak references;
+ // this is what keeps the VM open.
#[allow(dead_code)]
lua: Lua,
}
@@ -95,7 +108,6 @@ impl Parallax {
.with_context(|_| format!("loading {path}"))?
.ok_or_else(|| Error::ConfigRegistration {
path: path.to_owned(),
- what: "parallax",
})?;
let name = config
.setup
@@ -135,23 +147,47 @@ fn default_name(path: &Utf8Path) -> String {
.to_owned()
}
+/// Where `(parallax ...)` leaves its config. Closed once loading ends,
+/// so a call from inside a flow fails instead of registering into a
+/// slot nobody reads.
+enum Slot {
+ Empty,
+ Filled(Config),
+ Closed,
+}
+
+// Poisoning only follows a panic mid-store; the slot is still
+// coherent, so take it rather than fail.
+fn lock(slot: &Mutex<Slot>) -> MutexGuard<'_, Slot> {
+ slot.lock().unwrap_or_else(PoisonError::into_inner)
+}
+
/// Builds the `parallax` module in package.preload — a single function
/// that converts its argument to a Config and stores it in the slot
/// the caller keeps — and returns that slot. The function raises
/// errors while the config's stack frame is alive, so failures point
/// at the call site.
-fn install_module(lua: &Lua) -> Result<Arc<Mutex<Option<Config>>>, mlua::Error> {
- let slot: Arc<Mutex<Option<Config>>> = Arc::new(Mutex::new(None));
+fn install_module(lua: &Lua) -> Result<Arc<Mutex<Slot>>, mlua::Error> {
+ let slot = Arc::new(Mutex::new(Slot::Empty));
let parallax = {
let slot = slot.clone();
- lua.create_function(move |lua, config: Config| {
- // Poisoning only follows a panic mid-store; the slot is
- // still coherent, so take it rather than fail.
- let mut slot = slot.lock().unwrap_or_else(|poisoned| poisoned.into_inner());
- if slot.is_some() {
- return Err(error_at(lua, "parallax must be called exactly once"));
+ // Takes a Value and converts it here, not in the signature, so
+ // mlua doesn't wrap conversion errors in "bad argument #1".
+ lua.create_function(move |lua, value: Value| {
+ match *lock(&slot) {
+ Slot::Empty => {}
+ Slot::Filled(_) => {
+ return Err(error_at(lua, "parallax must be called exactly once"));
+ }
+ Slot::Closed => {
+ return Err(error_at(
+ lua,
+ "parallax can only be called while the config loads",
+ ));
+ }
}
- *slot = Some(config);
+ let config = Config::from_lua(value, lua)?;
+ *lock(&slot) = Slot::Filled(config);
Ok(())
})?
};
@@ -179,21 +215,27 @@ fn error_message(error: &mlua::Error) -> String {
}
/// Walks the Lua stack for the config's main chunk so errors point at
-/// the registration call site. Runs inside a registration callback,
-/// where that frame is still alive.
+/// the registration call site. After loading, that frame is gone, so
+/// it falls back to the innermost Lua frame.
fn caller_position(lua: &Lua) -> Option<String> {
- let debug: Table = lua.globals().get("debug").ok()?;
- let getinfo: Function = debug.get("getinfo").ok()?;
- let mut level: i64 = 2;
- loop {
- let info: Table = getinfo.call((level, "Sl")).ok()?;
- if info.get::<String>("what").ok()? == "main" {
- let source = info.get::<String>("short_src").ok()?;
- let line = info.get::<i64>("currentline").unwrap_or(0);
- return Some(format!("{source}:{line}"));
+ let mut innermost = None;
+ let mut level: usize = 0;
+ while let Some((what, position)) = lua.inspect_stack(level, frame_position) {
+ match what {
+ "main" => return position,
+ "Lua" if innermost.is_none() => innermost = position,
+ _ => {}
}
level = level.saturating_add(1);
}
+ innermost
+}
+
+fn frame_position(debug: &Debug) -> (&'static str, Option<String>) {
+ let source = debug.source();
+ let line = debug.current_line().unwrap_or(0);
+ let position = source.short_src.map(|src| format!("{src}:{line}"));
+ (source.what, position)
}
/// Loads the Fennel compiler, runs the config, and returns the config
@@ -204,12 +246,6 @@ fn run_config(lua: &Lua, source: &str, filename: &str) -> Result<Option<Config>,
let fennel: Table = lua.load(FENNEL_LUA).set_name("fennel.lua").eval()?;
let slot = install_module(lua)?;
- // Configs end with a registration call, which compiles to a tail
- // call; tail elimination destroys the config's stack frame, hiding
- // the call site from registration errors. Turning the final return
- // into a local assignment on the same line keeps the frame alive
- // without moving any other statement.
- //
// `correlate` makes Fennel emit each statement on its source line,
// so debug line numbers match the config; a form like the
// destructured require emits several statements and would otherwise
@@ -219,44 +255,42 @@ fn run_config(lua: &Lua, source: &str, filename: &str) -> Result<Option<Config>,
options.set("filename", filename)?;
options.set("correlate", true)?;
let compiled: String = compile.call((source, options))?;
- let compiled = protect_tail_call(&compiled);
lua.load(compiled)
.set_name(format!("@{filename}"))
.eval::<()>()?;
- Ok(slot
- .lock()
- .unwrap_or_else(|poisoned| poisoned.into_inner())
- .take())
+ match mem::replace(&mut *lock(&slot), Slot::Closed) {
+ Slot::Filled(config) => Ok(Some(config)),
+ Slot::Empty | Slot::Closed => Ok(None),
+ }
}
-// The index arithmetic in this helper is safe because `index` comes
-// from `rfind` on `compiled`: it's in bounds, non-zero only after the
-// `index == 0` check, and always followed by the "return " literal.
-#[allow(clippy::arithmetic_side_effects, clippy::indexing_slicing)]
-fn protect_tail_call(compiled: &str) -> String {
- // Fennel emits string literals escaped, so a newline before `return`
- // in compiled output is always a statement boundary.
- let at_line_start = |index: usize| index == 0 || compiled.as_bytes()[index - 1] == b'\n';
- let mut from = compiled.len();
- let mut found = None;
- while let Some(index) = compiled[..from].rfind("return ") {
- if at_line_start(index) {
- found = Some(index);
- break;
- }
- from = index;
- }
+#[cfg(test)]
+#[allow(clippy::expect_used)]
+mod tests {
+ use super::*;
- match found {
- None => compiled.to_owned(),
- Some(index) => {
- let mut protected = String::with_capacity(compiled.len() + 40);
- protected.push_str(&compiled[..index]);
- protected.push_str("local __parallax_return = ");
- protected.push_str(&compiled[index + "return ".len()..]);
- protected.push_str("\nreturn __parallax_return\n");
- protected
- }
+ #[test]
+ fn rejects_registration_after_loading() {
+ let lua = unsafe { Lua::unsafe_new() };
+ let source = concat!(
+ "(local parallax (require :parallax))\n",
+ "(fn late [] (parallax {:variants {:x {:viewport [1 1]}}",
+ " :server (fn [] nil) :flow (fn [] nil)}))\n",
+ "(parallax {:variants {:d {:viewport [1 1]}} :server (fn [] nil) :flow late})\n",
+ );
+ let config = run_config(&lua, source, "config.fnl")
+ .expect("config loads")
+ .expect("config registers");
+ let error = config
+ .flow
+ .call::<()>(())
+ .expect_err("registering after loading must fail");
+ assert!(
+ error
+ .to_string()
+ .contains("only be called while the config loads"),
+ "unexpected error: {error}"
+ );
}
}
diff --git a/src/spec.rs b/src/spec.rs
index 7716ead..955dea4 100644
--- a/src/spec.rs
+++ b/src/spec.rs
@@ -1,10 +1,14 @@
-#![cfg_attr(test, allow(clippy::expect_used, clippy::unwrap_used))]
+#![cfg_attr(
+ test,
+ allow(clippy::expect_used, clippy::indexing_slicing, clippy::unwrap_used)
+)]
use std::collections::HashMap;
use std::fmt;
use std::num::NonZeroU32;
use serde::Deserialize;
+use serde::de::IgnoredAny;
/// A string that rejected its own emptiness at deserialization time.
#[derive(Debug)]
@@ -52,9 +56,10 @@ pub struct Variant {
pub scheme: Scheme,
}
-/// The table passed to `(setup {...})`, with variants flattened into
-/// run order. Pure data — a function anywhere in it is an error, which
-/// is why it deserializes with mlua's default strict options.
+/// The data half of the table passed to `(parallax {...})`, with
+/// variants flattened into run order. The caller pulls `server` and
+/// `flow` out first; a function anywhere else is an error, which is why
+/// it deserializes with mlua's default strict options.
#[derive(Debug)]
#[allow(dead_code)] // baseline and name are read once running lands
pub(crate) struct Setup {
@@ -65,8 +70,8 @@ pub(crate) struct Setup {
pub(crate) name: Option<String>,
}
-/// What `(setup {...})` deserializes from: a table keyed by variant
-/// name, so the names are data rather than struct fields.
+/// What `(parallax {...})` deserializes from: variants keyed by name,
+/// so the names are data rather than struct fields.
#[derive(Deserialize)]
#[serde(deny_unknown_fields)]
struct SetupTable {
@@ -77,6 +82,14 @@ struct SetupTable {
#[serde(default)]
name: Option<String>,
+
+ // Already removed by the caller; declared so unknown-field errors
+ // list them among the expected keys.
+ #[serde(default, rename = "server")]
+ _server: Option<IgnoredAny>,
+
+ #[serde(default, rename = "flow")]
+ _flow: Option<IgnoredAny>,
}
#[derive(Deserialize)]
@@ -94,7 +107,11 @@ impl<'de> Deserialize<'de> for Setup {
variants,
baseline,
name,
+ ..
} = SetupTable::deserialize(deserializer)?;
+ if variants.is_empty() {
+ return Err(serde::de::Error::custom("variants must not be empty"));
+ }
let mut parsed = Vec::with_capacity(variants.len());
for (key, variant) in &variants {
let name = NonEmptyString::new(key.clone())
@@ -191,6 +208,11 @@ mod tests {
let cases = [
(r#"return {baseline = nil}"#, "missing field `variants`"),
(r#"return {variants = {}, extra = 1}"#, "unknown field"),
+ (r#"return {variants = {}}"#, "variants must not be empty"),
+ (
+ r#"return {variants = {desktop = {viewport = {1280, 900}}}, extra = 1}"#,
+ "`server`, `flow`",
+ ),
(
r#"return {variants = {desktop = {viewport = {1280, 900}, extra = 1}}}"#,
"unknown field",
diff --git a/tests/cli.rs b/tests/cli.rs
index 15f1e20..8bded2b 100644
--- a/tests/cli.rs
+++ b/tests/cli.rs
@@ -100,7 +100,29 @@ fn unknown_field_errors_point_at_the_parallax_call() {
.arg(&path)
.assert()
.failure()
- .stderr(predicate::str::contains("config.fnl:3:").and(predicate::str::contains("unknown")));
+ .stderr(
+ predicate::str::contains("config.fnl:3:")
+ .and(predicate::str::contains("unknown"))
+ .and(predicate::str::contains("`server`, `flow`"))
+ .and(predicate::str::contains("bad argument").not()),
+ );
+}
+
+#[test]
+fn leaves_the_config_table_intact() {
+ let (_dir, path) = config_file(concat!(
+ "(local parallax (require :parallax))\n",
+ "(local config {:variants {:desktop {:viewport [1280 900]}}\n",
+ " :server (fn [] {:cmd [\"true\"]}) :flow (fn [] nil)})\n",
+ "(parallax config)\n",
+ "(assert config.server \"server was removed from the config table\")\n",
+ ));
+ cargo_bin_cmd!("parallax")
+ .arg("list")
+ .arg("--config")
+ .arg(&path)
+ .assert()
+ .success();
}
#[test]