Skip to content

Commit a53ebdc

Browse files
committed
fix(auth): reuse a process-wide wasmtime engine
Create the wasmtime Engine lazily once per process and reuse it across OPAPolicy instances. This avoids repeatedly initializing JIT and trap-handling state as auth-enabled test clients come and go, which caused native xdist worker crashes without Python tracebacks. Remove the obsolete scoped-access-key worker pin while retaining coverage that revoked keys are reported as REVOKED and rejected by authentication and workspace endpoints. Signed-off-by: Ryan S <267728323+ironcommit@users.noreply.github.com>
1 parent 6f6c96b commit a53ebdc

6 files changed

Lines changed: 320 additions & 146 deletions

File tree

.github/workflows/ci.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1073,12 +1073,12 @@ jobs:
10731073
NMP_FILES_HF_RETRY_ATTEMPTS: "7"
10741074
NMP_FILES_HF_RETRY_INITIAL_DELAY_SECONDS: "1"
10751075
NMP_FILES_HF_RETRY_MAX_DELAY_SECONDS: "30"
1076+
PYTEST_EXTRA: "--capture=no -p no:faulthandler"
10761077
PYTEST_WORKERS: "4"
10771078
_TYPER_FORCE_DISABLE_TERMINAL: "1"
10781079
# Per-worker crash dumps; pytest's own faulthandler would point them at stderr, which
10791080
# xdist never forwards. See conftest.py.
10801081
PYTEST_CRASH_DUMP_DIR: crash-dumps
1081-
PYTEST_EXTRA: -p no:faulthandler
10821082
- name: Upload test artifacts
10831083
if: always()
10841084
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1

Makefile

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -382,12 +382,11 @@ test: test-unit ## Run all Python unit tests (fast tests without infrastructure
382382
PYTEST_VERBOSITY := $(if $(filter true,$(CI)),-q,-v)
383383
PYTEST_WORKERS ?= auto
384384
PYTEST_MAX_WORKERS ?= 16
385-
PYTEST_DIST ?= loadscope
386385
PYTEST_MAX_WORKER_RESTART ?= 2
387-
PYTEST_CMD = env PYTHONWARNINGS="ignore::UserWarning:pytest_only.version" $(UV) run --frozen \
386+
PYTEST_DIST ?= loadscope
387+
PYTEST_CMD = env PYTHONFAULTHANDLER=1 PYTHONWARNINGS="ignore::UserWarning:pytest_only.version" $(UV) run --frozen \
388388
pytest \
389-
-n $(PYTEST_WORKERS) --maxprocesses=$(PYTEST_MAX_WORKERS) \
390-
--max-worker-restart=$(PYTEST_MAX_WORKER_RESTART) \
389+
-n $(PYTEST_WORKERS) --maxprocesses=$(PYTEST_MAX_WORKERS) --max-worker-restart=$(PYTEST_MAX_WORKER_RESTART) \
391390
--dist $(PYTEST_DIST) --timeout=120 $(PYTEST_VERBOSITY) $(PYTEST_EXTRA)
392391

393392
PYTEST_CI_OPTS := --cov=src --cov=packages \
@@ -411,9 +410,10 @@ PYTEST_CI_CMD = timeout --kill-after=60s $(PYTEST_CI_TIMEOUT)s $(PYTEST_CMD)
411410
# ``loadscope``.
412411
test-integration test-integration-ci: PYTEST_DIST := loadgroup
413412

414-
# A killed integration worker never tears down its containers and ports, so replacing it re-runs
415-
# the group against resources the dead worker still holds. Unit workers own nothing external.
416-
test-integration test-integration-ci: PYTEST_MAX_WORKER_RESTART := 0
413+
# In CI, a crashed xdist worker should fail the job immediately. Restarting the
414+
# worker can hide the crashing test and leave pytest waiting until the wrapper
415+
# timeout kills the process.
416+
test-unit-ci test-integration-ci test-gpu-integration-ci: PYTEST_MAX_WORKER_RESTART := 0
417417

418418
.PHONY: test-unit
419419
test-unit: ## Run Python unit tests across all packages and services

services/core/auth/src/nmp/core/auth/app/embedded_pdp/engine.py

Lines changed: 151 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import json
77
import logging
88
import threading
9+
from functools import cache
910
from typing import Any, Dict, List, Optional, cast
1011

1112
from nmp.core.auth.app.embedded_pdp.policy_wasm import ensure_embedded_policy_wasm
@@ -23,13 +24,27 @@ class PolicyEngineError(Exception):
2324
"""Error during policy evaluation."""
2425

2526

27+
@cache
28+
def _get_engine() -> Engine:
29+
"""Return the process-wide wasmtime Engine.
30+
31+
Engine setup is the heavyweight step in wasmtime — JIT code generation and process-wide
32+
trap/signal-handling registration — and wasmtime's own guidance is one Engine per process with
33+
many cheap Stores created from it. Creating a fresh Engine per thread-local OPAPolicy would
34+
churn through many Engines over an xdist worker's lifetime; that churn, not any single test, is
35+
what was crashing workers with no traceback ("node down: Not properly terminated").
36+
"""
37+
config = Config()
38+
config.consume_fuel = True
39+
return Engine(config)
40+
41+
2642
class OPAPolicy:
2743
"""Wrapper for OPA WASM policy evaluation."""
2844

2945
def __init__(self, wasm_path: str, *, fuel_limit: int = 200_000_000, memory_limit_mb: int = 32):
30-
config = Config()
31-
config.consume_fuel = True
32-
engine = Engine(config)
46+
self._owner_thread_id = threading.get_ident()
47+
engine = _get_engine()
3348

3449
self.fuel_limit = fuel_limit
3550
self.store = Store(engine)
@@ -73,20 +88,29 @@ def __init__(self, wasm_path: str, *, fuel_limit: int = 200_000_000, memory_limi
7388
self._base_heap = self._export_func("opa_heap_ptr_get")(self.store)
7489
self._data_heap = self._base_heap
7590
self._data_addr: Optional[int] = None
76-
self._lock = threading.Lock()
91+
92+
def _assert_owner_thread(self) -> None:
93+
current_thread_id = threading.get_ident()
94+
if current_thread_id != self._owner_thread_id:
95+
raise RuntimeError(
96+
f"OPAPolicy used from a different thread (owner={self._owner_thread_id}, current={current_thread_id})"
97+
)
7798

7899
def _export_func(self, name: str) -> Func:
100+
self._assert_owner_thread()
79101
return cast(Func, self.exports[name])
80102

81103
def _write_json(self, data: Any) -> int:
82104
"""Write JSON to WASM memory, return OPA value address."""
105+
self._assert_owner_thread()
83106
json_bytes = json.dumps(data).encode("utf-8")
84107
addr = self._export_func("opa_malloc")(self.store, len(json_bytes))
85108
self.memory.write(self.store, json_bytes, addr)
86109
return self._export_func("opa_json_parse")(self.store, addr, len(json_bytes))
87110

88111
def _read_json(self, addr: int) -> Any:
89112
"""Read OPA value as JSON from WASM memory."""
113+
self._assert_owner_thread()
90114
json_addr = self._export_func("opa_json_dump")(self.store, addr)
91115
mem = self.memory.data_ptr(self.store)
92116
end = json_addr
@@ -96,74 +120,151 @@ def _read_json(self, addr: int) -> Any:
96120

97121
def set_data(self, data: Dict[str, Any]) -> None:
98122
"""Set the base data document."""
99-
with self._lock:
100-
self.store.set_fuel(DATA_LOADING_FUEL)
101-
self._export_func("opa_heap_ptr_set")(self.store, self._base_heap)
102-
self._data_addr = self._write_json(data)
103-
self._data_heap = self._export_func("opa_heap_ptr_get")(self.store)
123+
self._assert_owner_thread()
124+
self.store.set_fuel(DATA_LOADING_FUEL)
125+
self._export_func("opa_heap_ptr_set")(self.store, self._base_heap)
126+
self._data_addr = self._write_json(data)
127+
self._data_heap = self._export_func("opa_heap_ptr_get")(self.store)
104128

105129
def evaluate(self, input_data: Dict[str, Any], entrypoint: int = 0) -> Any:
106130
"""Evaluate policy with given input."""
131+
self._assert_owner_thread()
107132
if self._data_addr is None:
108133
raise PolicyEngineError("Policy data not loaded — refusing to evaluate (fail-closed)")
109134

135+
self.store.set_fuel(self.fuel_limit)
136+
137+
heap_base = getattr(self, "_data_heap", self._base_heap)
138+
self._export_func("opa_heap_ptr_set")(self.store, heap_base)
139+
140+
ctx = self._export_func("opa_eval_ctx_new")(self.store)
141+
self._export_func("opa_eval_ctx_set_input")(self.store, ctx, self._write_json(input_data))
142+
self._export_func("opa_eval_ctx_set_data")(self.store, ctx, self._data_addr)
143+
self._export_func("opa_eval_ctx_set_entrypoint")(self.store, ctx, entrypoint)
144+
145+
self._export_func("eval")(self.store, ctx)
146+
return self._read_json(self._export_func("opa_eval_ctx_get_result")(self.store, ctx))
147+
148+
149+
class _PolicyRuntimeManager:
150+
"""Owns policy data snapshots and thread-local WASM policy runtimes."""
151+
152+
def __init__(self) -> None:
153+
self._local = threading.local()
154+
self._lock = threading.Lock()
155+
self._data: Dict[str, Any] = {}
156+
self._data_loaded = False
157+
self._data_version = 0
158+
self._generation = 0
159+
160+
def _clear_thread_policy(self) -> None:
161+
for attr in ("policy", "policy_generation", "policy_data_version"):
162+
if hasattr(self._local, attr):
163+
delattr(self._local, attr)
164+
165+
def _create_policy(self) -> OPAPolicy:
166+
from nmp.common.config import get_service_config
167+
from nmp.core.auth.config import AuthServiceConfig
168+
169+
cfg = get_service_config(AuthServiceConfig)
170+
path = ensure_embedded_policy_wasm(auto_build=cfg.embedded_pdp_auto_build_wasm)
171+
return OPAPolicy(
172+
str(path),
173+
fuel_limit=cfg.embedded_pdp_cpu_limit * 1_000_000,
174+
memory_limit_mb=cfg.embedded_pdp_memory_limit_mb,
175+
)
176+
177+
def _data_snapshot(self) -> tuple[Dict[str, Any], int, bool]:
110178
with self._lock:
111-
self.store.set_fuel(self.fuel_limit)
179+
return self._data, self._data_version, self._data_loaded
112180

113-
heap_base = getattr(self, "_data_heap", self._base_heap)
114-
self._export_func("opa_heap_ptr_set")(self.store, heap_base)
181+
def _generation_snapshot(self) -> int:
182+
with self._lock:
183+
return self._generation
184+
185+
def _get_thread_policy(self) -> Optional[OPAPolicy]:
186+
return cast(Optional[OPAPolicy], getattr(self._local, "policy", None))
187+
188+
def _sync_data_if_needed(self, policy: OPAPolicy) -> None:
189+
while True:
190+
local_version = cast(int, getattr(self._local, "policy_data_version", -1))
191+
data, data_version, data_loaded = self._data_snapshot()
192+
if local_version == data_version:
193+
return
194+
195+
if data_loaded:
196+
policy.set_data(data)
197+
198+
_, current_data_version, _ = self._data_snapshot()
199+
if data_version == current_data_version:
200+
self._local.policy_data_version = data_version
201+
return
202+
203+
def get_policy(self) -> OPAPolicy:
204+
"""Get or create the current thread's policy runtime."""
205+
while True:
206+
generation = self._generation_snapshot()
207+
policy = self._get_thread_policy()
208+
policy_generation = cast(int, getattr(self._local, "policy_generation", -1))
209+
if policy is None or policy_generation != generation:
210+
policy = self._create_policy()
211+
self._local.policy = policy
212+
self._local.policy_generation = generation
213+
self._local.policy_data_version = -1
214+
215+
self._sync_data_if_needed(policy)
216+
if generation == self._generation_snapshot():
217+
return policy
115218

116-
ctx = self._export_func("opa_eval_ctx_new")(self.store)
117-
self._export_func("opa_eval_ctx_set_input")(self.store, ctx, self._write_json(input_data))
118-
self._export_func("opa_eval_ctx_set_data")(self.store, ctx, self._data_addr)
119-
self._export_func("opa_eval_ctx_set_entrypoint")(self.store, ctx, entrypoint)
219+
def set_data(self, data: Dict[str, Any]) -> None:
220+
"""Set policy data (principals, roles, etc.)."""
221+
with self._lock:
222+
self._data = data
223+
self._data_loaded = True
224+
self._data_version += 1
120225

121-
self._export_func("eval")(self.store, ctx)
122-
return self._read_json(self._export_func("opa_eval_ctx_get_result")(self.store, ctx))
226+
policy = self._get_thread_policy()
227+
if policy is not None and getattr(self._local, "policy_generation", -1) == self._generation_snapshot():
228+
self._sync_data_if_needed(policy)
123229

230+
def reload(self) -> None:
231+
"""Force each thread to rebuild its policy runtime on next access."""
232+
with self._lock:
233+
self._generation += 1
234+
self._clear_thread_policy()
235+
self.get_policy()
124236

125-
# Module-level singleton
126-
_policy: Optional[OPAPolicy] = None
127-
_policy_lock = threading.Lock()
128-
_policy_data: Dict[str, Any] = {}
237+
def reset_for_testing(self) -> None:
238+
"""Reset policy runtime state between tests."""
239+
with self._lock:
240+
self._data = {}
241+
self._data_loaded = False
242+
self._data_version += 1
243+
self._generation += 1
244+
self._clear_thread_policy()
245+
246+
247+
_policy_runtime = _PolicyRuntimeManager()
129248

130249

131250
def get_policy() -> OPAPolicy:
132-
"""Get or create the singleton policy instance (thread-safe, double-checked locking)."""
133-
global _policy
134-
if _policy is None:
135-
with _policy_lock:
136-
if _policy is None:
137-
from nmp.common.config import get_service_config
138-
from nmp.core.auth.config import AuthServiceConfig
139-
140-
cfg = get_service_config(AuthServiceConfig)
141-
path = ensure_embedded_policy_wasm(auto_build=cfg.embedded_pdp_auto_build_wasm)
142-
_policy = OPAPolicy(
143-
str(path),
144-
fuel_limit=cfg.embedded_pdp_cpu_limit * 1_000_000,
145-
memory_limit_mb=cfg.embedded_pdp_memory_limit_mb,
146-
)
147-
if _policy_data:
148-
_policy.set_data(_policy_data)
149-
return _policy
251+
"""Get or create the current thread's policy runtime."""
252+
return _policy_runtime.get_policy()
150253

151254

152255
def set_policy_data(data: Dict[str, Any]) -> None:
153256
"""Set policy data (principals, roles, etc.)."""
154-
global _policy_data
155-
with _policy_lock:
156-
_policy_data = data
157-
if _policy is not None:
158-
_policy.set_data(data)
257+
_policy_runtime.set_data(data)
159258

160259

161260
def reload_policy() -> None:
162-
"""Force reload the policy."""
163-
global _policy
164-
with _policy_lock:
165-
_policy = None
166-
get_policy()
261+
"""Force each thread to rebuild its policy runtime on next access."""
262+
_policy_runtime.reload()
263+
264+
265+
def _reset_policy_state_for_testing() -> None:
266+
"""Reset module policy state between tests."""
267+
_policy_runtime.reset_for_testing()
167268

168269

169270
def evaluate(entrypoint: str, input_data: Dict[str, Any]) -> Dict[str, Any]:

0 commit comments

Comments
 (0)