mirror of
https://github.com/logos-messaging/nim-ffi.git
synced 2026-07-16 21:10:19 +00:00
feat(ffi): RET_STALE_WARN progress callback replacing handler timeout (#129)
This commit is contained in:
parent
e0faefd5a0
commit
7b028e64c4
21
CHANGELOG.md
21
CHANGELOG.md
@ -42,16 +42,17 @@ All notable changes to this project are documented in this file.
|
||||
`nimble genbindings_c` / `genbindings_c_echo` / `check_bindings_c` /
|
||||
`test_c_e2e` tasks, a `tests/e2e/c` ctest harness, and a
|
||||
`tests/unit/test_c_codegen.nim` unit suite.
|
||||
- Configurable per-request handler timeout with a finite default: each
|
||||
`FFIContext` now carries a `defaultRequestTimeout` (5s) applied to every
|
||||
handler, replacing the previous unbounded wait so a wedged handler can no
|
||||
longer hang a foreign caller forever. On trip the caller is unblocked with an
|
||||
`ffi request timed out after <n>ms` err; the handler is left running (not
|
||||
cancelled, since a hard-cancel mid-call into the underlying library can leave
|
||||
it partial), and the callback still fires exactly once. Override per proc with
|
||||
a `"timeout = <ms>"` spec (e.g. `{.ffi: "timeout = 30000".}`), parsed like the
|
||||
`abi = ...` spec; runtime-only, codegen ignores it
|
||||
([#93](https://github.com/logos-messaging/nim-ffi/issues/93)).
|
||||
- Non-terminal `RET_STALE_WARN` (3) progress callback in place of a handler
|
||||
timeout: nim-ffi never times a handler out (a hard-cancel mid-call into the
|
||||
underlying library can leave it half-applied). Instead, while a request is
|
||||
still in flight its result callback receives a `RET_STALE_WARN` every 5s
|
||||
(Android's ANR interval; override with `-d:ffiStaleWarnIntervalMs=<ms>`), with
|
||||
the payload carrying the elapsed milliseconds as a decimal string. The request
|
||||
always ends with exactly one terminal `RET_OK` / `RET_ERR`; the dev decides
|
||||
what to do with a slow one. Replaces the never-released per-proc
|
||||
`{.ffi: "timeout = <ms>".}` override and the `defaultRequestTimeout` context
|
||||
field ([#126](https://github.com/logos-messaging/nim-ffi/issues/126),
|
||||
supersedes [#93](https://github.com/logos-messaging/nim-ffi/issues/93)).
|
||||
- Per-interaction ABI-format annotations: `declareLibrary` now takes an
|
||||
optional `defaultABIFormat` (`"cbor"` default, or `"c"`) that every
|
||||
`{.ffi.}` / `{.ffiCtor.}` / `{.ffiDtor.}` / `{.ffiRaw.}` / `{.ffiEvent.}`
|
||||
|
||||
39
README.md
39
README.md
@ -91,26 +91,33 @@ Every `{.ffi.}` / `{.ffiCtor.}` proc must have an explicit
|
||||
`return ok(...)` without awaiting). The `Result`'s error string is delivered to
|
||||
the foreign caller as the failure message.
|
||||
|
||||
### Request timeouts
|
||||
### The result callback contract
|
||||
|
||||
Every handler runs under a deadline. The default is `DefaultRequestTimeout`
|
||||
(5s, `ffi/ffi_context.nim`), applied to every proc so a wedged handler can't
|
||||
hang a foreign caller forever. On trip the caller is unblocked with an `ffi
|
||||
request timed out after <n>ms` error; the handler is **not** cancelled — a
|
||||
hard cancel mid-call into the underlying library can leave it half-applied — so
|
||||
it keeps running, and the caller's callback still fires exactly once.
|
||||
Each request carries a result callback. It receives one of these status codes
|
||||
(`ret` / `err_code`):
|
||||
|
||||
Raise or lower the deadline per proc with a `"timeout = <ms>"` spec, parsed
|
||||
like the `abi = ...` spec below:
|
||||
| Code | Value | Terminal? | Meaning |
|
||||
| --- | --- | --- | --- |
|
||||
| `RET_OK` | 0 | yes | Success; the payload carries the encoded result. |
|
||||
| `RET_ERR` | 1 | yes | Failure; the payload carries the UTF-8 error string. |
|
||||
| `RET_MISSING_CALLBACK` | 2 | — | No callback was passed; the request path reports this itself. |
|
||||
| `RET_STALE_WARN` | 3 | **no** | Progress ping — the handler is still running. |
|
||||
|
||||
```nim
|
||||
proc slowOp*(
|
||||
c: Counter, req: BumpRequest
|
||||
): Future[Result[BumpResponse, string]] {.ffi: "timeout = 30000".} =
|
||||
...
|
||||
```
|
||||
**nim-ffi never times a handler out.** A slow request runs to its natural
|
||||
`RET_OK` / `RET_ERR`; it is never cancelled (a hard-cancel mid-call into the
|
||||
underlying library can leave it half-applied). Instead, while a handler is still
|
||||
in flight the callback receives a **non-terminal** `RET_STALE_WARN` every 5s
|
||||
(Android's ANR interval; override at build time with
|
||||
`-d:ffiStaleWarnIntervalMs=<ms>`), with the payload carrying the elapsed
|
||||
milliseconds as a decimal string. The dev decides what to do with a slow request
|
||||
— keep waiting, surface a spinner, tear the context down — nim-ffi does not
|
||||
decide for them.
|
||||
|
||||
The timeout is runtime-only; binding codegen ignores it.
|
||||
`RET_STALE_WARN` may fire any number of times and is **always** followed by
|
||||
exactly one terminal `RET_OK` / `RET_ERR`. A caller that only wants the final
|
||||
answer must ignore it (do not treat a non-zero code as an error without checking
|
||||
for `RET_STALE_WARN` first). The generated higher-level typed wrappers currently
|
||||
ignore it; the progress signal is delivered at the raw result-callback boundary.
|
||||
|
||||
### Events
|
||||
|
||||
|
||||
@ -9,6 +9,10 @@
|
||||
#define NIMFFI_RET_OK 0
|
||||
#define NIMFFI_RET_ERR 1
|
||||
#define NIMFFI_RET_MISSING_CALLBACK 2
|
||||
/* Non-terminal: the request is still running. Fires every ~5s with `msg`
|
||||
carrying the elapsed milliseconds as decimal text; always followed by a
|
||||
terminal RET_OK/RET_ERR. Ignore it unless you want progress. */
|
||||
#define NIMFFI_RET_STALE_WARN 3
|
||||
|
||||
/* Flat wire structs — the C ABI. Strings are borrowed, NUL-terminated
|
||||
`const char*` valid only for the duration of the call they cross. */
|
||||
@ -54,6 +58,8 @@ typedef struct { EchoCreateFn fn; void* user_data; } EchoCreateBox;
|
||||
static void echo_create_trampoline(int ret, const char* ctx_addr, const char* err_msg, void* ud) {
|
||||
EchoCreateBox* box = (EchoCreateBox*)ud;
|
||||
if (!box) return;
|
||||
/* Non-terminal progress ping: keep the box for the terminal reply. */
|
||||
if (ret == NIMFFI_RET_STALE_WARN) return;
|
||||
if (!box->fn) { free(box); return; }
|
||||
if (ret != 0) {
|
||||
box->fn(ret, NULL, err_msg ? err_msg : "FFI create failed", box->user_data);
|
||||
|
||||
@ -222,6 +222,8 @@ typedef void (*EchoCreateFn)(int err_code, EchoCtx* ctx, const char* err_msg, vo
|
||||
typedef struct { EchoCreateFn fn; void* user_data; } EchoCreateBox;
|
||||
static void echo_create_trampoline(int ret, const char* msg, size_t len, void* ud) {
|
||||
EchoCreateBox* box = (EchoCreateBox*)ud;
|
||||
/* Non-terminal progress ping: keep the box for the terminal reply. */
|
||||
if (ret == NIMFFI_RET_STALE_WARN) return;
|
||||
if (!box->fn) {
|
||||
free(box);
|
||||
return;
|
||||
@ -297,6 +299,8 @@ typedef void (*EchoShoutReplyFn)(int err_code, const ShoutResponse* reply, const
|
||||
typedef struct { EchoShoutReplyFn fn; void* user_data; } EchoShoutCallBox;
|
||||
static void echo_shout_reply_trampoline(int ret, const char* msg, size_t len, void* ud) {
|
||||
EchoShoutCallBox* box = (EchoShoutCallBox*)ud;
|
||||
/* Non-terminal progress ping: keep the box for the terminal reply. */
|
||||
if (ret == NIMFFI_RET_STALE_WARN) return;
|
||||
if (!box->fn) {
|
||||
free(box);
|
||||
return;
|
||||
@ -357,6 +361,8 @@ typedef void (*EchoVersionReplyFn)(int err_code, const NimFfiStr* reply, const c
|
||||
typedef struct { EchoVersionReplyFn fn; void* user_data; } EchoVersionCallBox;
|
||||
static void echo_version_reply_trampoline(int ret, const char* msg, size_t len, void* ud) {
|
||||
EchoVersionCallBox* box = (EchoVersionCallBox*)ud;
|
||||
/* Non-terminal progress ping: keep the box for the terminal reply. */
|
||||
if (ret == NIMFFI_RET_STALE_WARN) return;
|
||||
if (!box->fn) {
|
||||
free(box);
|
||||
return;
|
||||
|
||||
@ -20,10 +20,17 @@ typedef void (*FFICallback)(int ret, const char* msg, size_t len, void* user_dat
|
||||
* value handed to a result callback's `err_code` (or returned by a submit call)
|
||||
* is a failure. NIMFFI_RET_MISSING_CALLBACK is a special case from the Nim
|
||||
* dispatcher: the callback will never fire, so the request path must report the
|
||||
* failure itself. */
|
||||
* failure itself.
|
||||
*
|
||||
* NIMFFI_RET_STALE_WARN is the one NON-terminal code: nim-ffi delivers it every
|
||||
* ~5s while a handler is still running (with `msg`/`len` carrying the elapsed
|
||||
* milliseconds as decimal text), then still ends with a terminal RET_OK/RET_ERR.
|
||||
* A caller that only wants the final answer must ignore it, not treat it as an
|
||||
* error. */
|
||||
#define NIMFFI_RET_OK 0
|
||||
#define NIMFFI_RET_ERROR 1
|
||||
#define NIMFFI_RET_MISSING_CALLBACK 2
|
||||
#define NIMFFI_RET_STALE_WARN 3
|
||||
|
||||
/* ── leaf encoders ─────────────────────────────────────────────────────── */
|
||||
static inline CborError nimffi_enc_bool(CborEncoder* e, const bool* v) {
|
||||
|
||||
@ -29,6 +29,16 @@ extern "C" {
|
||||
#include <tinycbor/cbor.h>
|
||||
}
|
||||
|
||||
// nim-ffi result-callback status codes (mirror ffi/ffi_types.nim and the C
|
||||
// header). Guarded so a translation unit that also pulls in the C header keeps
|
||||
// a single definition.
|
||||
#ifndef NIMFFI_RET_OK
|
||||
#define NIMFFI_RET_OK 0
|
||||
#define NIMFFI_RET_ERR 1
|
||||
#define NIMFFI_RET_MISSING_CALLBACK 2
|
||||
#define NIMFFI_RET_STALE_WARN 3
|
||||
#endif
|
||||
|
||||
// ============================================================
|
||||
// Result<T> — exception-free error channel
|
||||
// ============================================================
|
||||
@ -448,6 +458,12 @@ struct FFICallState_ {
|
||||
};
|
||||
|
||||
inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) {
|
||||
// NIMFFI_RET_STALE_WARN (3) is a non-terminal progress ping: the request is
|
||||
// still running. This blocking wrapper only reports the final result, so
|
||||
// ignore it WITHOUT touching `ud` — a terminal callback still owns the
|
||||
// shared handle and will free it.
|
||||
if (ret == NIMFFI_RET_STALE_WARN) return;
|
||||
|
||||
// ffi_call_ heap-allocated a shared_ptr and passed its address as ud;
|
||||
// take ownership here so it's freed on every exit path.
|
||||
std::unique_ptr<std::shared_ptr<FFICallState_>> handle(
|
||||
@ -455,7 +471,7 @@ inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) {
|
||||
FFICallState_& s = **handle;
|
||||
|
||||
std::lock_guard<std::mutex> lock(s.mtx);
|
||||
s.ok = (ret == 0);
|
||||
s.ok = (ret == NIMFFI_RET_OK);
|
||||
if (msg && len > 0) {
|
||||
const auto* p = reinterpret_cast<const std::uint8_t*>(msg);
|
||||
if (s.ok) s.bytes.assign(p, p + len);
|
||||
@ -472,7 +488,7 @@ inline Result<std::vector<std::uint8_t>> ffi_call_(
|
||||
auto state = std::make_shared<FFICallState_>();
|
||||
auto* cb_ref = new std::shared_ptr<FFICallState_>(state);
|
||||
const int ret = f(ffi_cb_, cb_ref);
|
||||
if (ret == 2) {
|
||||
if (ret == NIMFFI_RET_MISSING_CALLBACK) {
|
||||
delete cb_ref;
|
||||
return Result<Bytes>::err("RET_MISSING_CALLBACK (internal error)");
|
||||
}
|
||||
|
||||
@ -829,6 +829,8 @@ typedef void (*MyTimerCreateFn)(int err_code, MyTimerCtx* ctx, const char* err_m
|
||||
typedef struct { MyTimerCreateFn fn; void* user_data; } MyTimerCreateBox;
|
||||
static void my_timer_create_trampoline(int ret, const char* msg, size_t len, void* ud) {
|
||||
MyTimerCreateBox* box = (MyTimerCreateBox*)ud;
|
||||
/* Non-terminal progress ping: keep the box for the terminal reply. */
|
||||
if (ret == NIMFFI_RET_STALE_WARN) return;
|
||||
if (!box->fn) {
|
||||
free(box);
|
||||
return;
|
||||
@ -940,6 +942,8 @@ typedef void (*MyTimerEchoReplyFn)(int err_code, const EchoResponse* reply, cons
|
||||
typedef struct { MyTimerEchoReplyFn fn; void* user_data; } MyTimerEchoCallBox;
|
||||
static void my_timer_echo_reply_trampoline(int ret, const char* msg, size_t len, void* ud) {
|
||||
MyTimerEchoCallBox* box = (MyTimerEchoCallBox*)ud;
|
||||
/* Non-terminal progress ping: keep the box for the terminal reply. */
|
||||
if (ret == NIMFFI_RET_STALE_WARN) return;
|
||||
if (!box->fn) {
|
||||
free(box);
|
||||
return;
|
||||
@ -1000,6 +1004,8 @@ typedef void (*MyTimerVersionReplyFn)(int err_code, const NimFfiStr* reply, cons
|
||||
typedef struct { MyTimerVersionReplyFn fn; void* user_data; } MyTimerVersionCallBox;
|
||||
static void my_timer_version_reply_trampoline(int ret, const char* msg, size_t len, void* ud) {
|
||||
MyTimerVersionCallBox* box = (MyTimerVersionCallBox*)ud;
|
||||
/* Non-terminal progress ping: keep the box for the terminal reply. */
|
||||
if (ret == NIMFFI_RET_STALE_WARN) return;
|
||||
if (!box->fn) {
|
||||
free(box);
|
||||
return;
|
||||
@ -1059,6 +1065,8 @@ typedef void (*MyTimerComplexReplyFn)(int err_code, const ComplexResponse* reply
|
||||
typedef struct { MyTimerComplexReplyFn fn; void* user_data; } MyTimerComplexCallBox;
|
||||
static void my_timer_complex_reply_trampoline(int ret, const char* msg, size_t len, void* ud) {
|
||||
MyTimerComplexCallBox* box = (MyTimerComplexCallBox*)ud;
|
||||
/* Non-terminal progress ping: keep the box for the terminal reply. */
|
||||
if (ret == NIMFFI_RET_STALE_WARN) return;
|
||||
if (!box->fn) {
|
||||
free(box);
|
||||
return;
|
||||
@ -1119,6 +1127,8 @@ typedef void (*MyTimerScheduleReplyFn)(int err_code, const ScheduleResult* reply
|
||||
typedef struct { MyTimerScheduleReplyFn fn; void* user_data; } MyTimerScheduleCallBox;
|
||||
static void my_timer_schedule_reply_trampoline(int ret, const char* msg, size_t len, void* ud) {
|
||||
MyTimerScheduleCallBox* box = (MyTimerScheduleCallBox*)ud;
|
||||
/* Non-terminal progress ping: keep the box for the terminal reply. */
|
||||
if (ret == NIMFFI_RET_STALE_WARN) return;
|
||||
if (!box->fn) {
|
||||
free(box);
|
||||
return;
|
||||
|
||||
@ -20,10 +20,17 @@ typedef void (*FFICallback)(int ret, const char* msg, size_t len, void* user_dat
|
||||
* value handed to a result callback's `err_code` (or returned by a submit call)
|
||||
* is a failure. NIMFFI_RET_MISSING_CALLBACK is a special case from the Nim
|
||||
* dispatcher: the callback will never fire, so the request path must report the
|
||||
* failure itself. */
|
||||
* failure itself.
|
||||
*
|
||||
* NIMFFI_RET_STALE_WARN is the one NON-terminal code: nim-ffi delivers it every
|
||||
* ~5s while a handler is still running (with `msg`/`len` carrying the elapsed
|
||||
* milliseconds as decimal text), then still ends with a terminal RET_OK/RET_ERR.
|
||||
* A caller that only wants the final answer must ignore it, not treat it as an
|
||||
* error. */
|
||||
#define NIMFFI_RET_OK 0
|
||||
#define NIMFFI_RET_ERROR 1
|
||||
#define NIMFFI_RET_MISSING_CALLBACK 2
|
||||
#define NIMFFI_RET_STALE_WARN 3
|
||||
|
||||
/* ── leaf encoders ─────────────────────────────────────────────────────── */
|
||||
static inline CborError nimffi_enc_bool(CborEncoder* e, const bool* v) {
|
||||
|
||||
@ -29,6 +29,16 @@ extern "C" {
|
||||
#include <tinycbor/cbor.h>
|
||||
}
|
||||
|
||||
// nim-ffi result-callback status codes (mirror ffi/ffi_types.nim and the C
|
||||
// header). Guarded so a translation unit that also pulls in the C header keeps
|
||||
// a single definition.
|
||||
#ifndef NIMFFI_RET_OK
|
||||
#define NIMFFI_RET_OK 0
|
||||
#define NIMFFI_RET_ERR 1
|
||||
#define NIMFFI_RET_MISSING_CALLBACK 2
|
||||
#define NIMFFI_RET_STALE_WARN 3
|
||||
#endif
|
||||
|
||||
#include <unordered_map>
|
||||
// ============================================================
|
||||
// Result<T> — exception-free error channel
|
||||
@ -748,6 +758,12 @@ struct FFICallState_ {
|
||||
};
|
||||
|
||||
inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) {
|
||||
// NIMFFI_RET_STALE_WARN (3) is a non-terminal progress ping: the request is
|
||||
// still running. This blocking wrapper only reports the final result, so
|
||||
// ignore it WITHOUT touching `ud` — a terminal callback still owns the
|
||||
// shared handle and will free it.
|
||||
if (ret == NIMFFI_RET_STALE_WARN) return;
|
||||
|
||||
// ffi_call_ heap-allocated a shared_ptr and passed its address as ud;
|
||||
// take ownership here so it's freed on every exit path.
|
||||
std::unique_ptr<std::shared_ptr<FFICallState_>> handle(
|
||||
@ -755,7 +771,7 @@ inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) {
|
||||
FFICallState_& s = **handle;
|
||||
|
||||
std::lock_guard<std::mutex> lock(s.mtx);
|
||||
s.ok = (ret == 0);
|
||||
s.ok = (ret == NIMFFI_RET_OK);
|
||||
if (msg && len > 0) {
|
||||
const auto* p = reinterpret_cast<const std::uint8_t*>(msg);
|
||||
if (s.ok) s.bytes.assign(p, p + len);
|
||||
@ -772,7 +788,7 @@ inline Result<std::vector<std::uint8_t>> ffi_call_(
|
||||
auto state = std::make_shared<FFICallState_>();
|
||||
auto* cb_ref = new std::shared_ptr<FFICallState_>(state);
|
||||
const int ret = f(ffi_cb_, cb_ref);
|
||||
if (ret == 2) {
|
||||
if (ret == NIMFFI_RET_MISSING_CALLBACK) {
|
||||
delete cb_ref;
|
||||
return Result<Bytes>::err("RET_MISSING_CALLBACK (internal error)");
|
||||
}
|
||||
|
||||
@ -31,16 +31,26 @@ unsafe fn ffi_payload(ret: c_int, msg: *const c_char, len: usize) -> FFIResult {
|
||||
} else {
|
||||
slice::from_raw_parts(msg as *const u8, len).to_vec()
|
||||
};
|
||||
if ret == 0 { Ok(bytes) }
|
||||
if ret == NIMFFI_RET_OK { Ok(bytes) }
|
||||
else { Err(String::from_utf8_lossy(&bytes).into_owned()) }
|
||||
}
|
||||
|
||||
// nim-ffi result-callback status codes (mirror ffi/ffi_types.nim).
|
||||
const NIMFFI_RET_OK: c_int = 0;
|
||||
const NIMFFI_RET_MISSING_CALLBACK: c_int = 2;
|
||||
const NIMFFI_RET_STALE_WARN: c_int = 3;
|
||||
|
||||
unsafe extern "C" fn on_result(
|
||||
ret: c_int,
|
||||
msg: *const c_char,
|
||||
len: usize,
|
||||
user_data: *mut c_void,
|
||||
) {
|
||||
// NIMFFI_RET_STALE_WARN (3) is a non-terminal progress ping: the request
|
||||
// is still running. This wrapper only delivers the final result, so ignore
|
||||
// it WITHOUT reclaiming the box — a terminal callback still owns the Sender.
|
||||
if ret == NIMFFI_RET_STALE_WARN { return; }
|
||||
|
||||
// Take ownership of the boxed Sender — dropping it at end of scope
|
||||
// releases the only outstanding handle.
|
||||
let tx = Box::from_raw(user_data as *mut FFISender);
|
||||
@ -66,7 +76,7 @@ where
|
||||
let (tx, rx) = flume::bounded::<FFIResult>(1);
|
||||
let raw = Box::into_raw(Box::new(tx)) as *mut c_void;
|
||||
let ret = f(on_result, raw);
|
||||
if ret == 2 {
|
||||
if ret == NIMFFI_RET_MISSING_CALLBACK {
|
||||
// Callback will never fire; reclaim the box to avoid a leak.
|
||||
drop(unsafe { Box::from_raw(raw as *mut FFISender) });
|
||||
return Err("RET_MISSING_CALLBACK (internal error)".into());
|
||||
@ -87,7 +97,7 @@ where
|
||||
let (tx, rx) = flume::bounded::<FFIResult>(1);
|
||||
let raw = Box::into_raw(Box::new(tx)) as *mut c_void;
|
||||
let ret = f(on_result, raw);
|
||||
if ret == 2 {
|
||||
if ret == NIMFFI_RET_MISSING_CALLBACK {
|
||||
drop(unsafe { Box::from_raw(raw as *mut FFISender) });
|
||||
return Err("RET_MISSING_CALLBACK (internal error)".into());
|
||||
}
|
||||
|
||||
@ -433,6 +433,10 @@ proc emitReplyTrampolineHead(lines: var seq[string], tramp, boxType, fallback: s
|
||||
"static void " & tramp & "(int ret, const char* msg, size_t len, void* ud) {"
|
||||
)
|
||||
lines.add(" " & boxType & "* box = (" & boxType & "*)ud;")
|
||||
lines.add(
|
||||
" /* Non-terminal progress ping: keep the box for the terminal reply. */"
|
||||
)
|
||||
lines.add(" if (ret == NIMFFI_RET_STALE_WARN) return;")
|
||||
lines.add(" if (!box->fn) {")
|
||||
lines.add(" free(box);")
|
||||
lines.add(" return;")
|
||||
|
||||
@ -271,6 +271,10 @@ proc emitCtxAndCtor(
|
||||
)
|
||||
lines.add(" " & createBox & "* box = (" & createBox & "*)ud;")
|
||||
lines.add(" if (!box) return;")
|
||||
lines.add(
|
||||
" /* Non-terminal progress ping: keep the box for the terminal reply. */"
|
||||
)
|
||||
lines.add(" if (ret == NIMFFI_RET_STALE_WARN) return;")
|
||||
lines.add(" if (!box->fn) { free(box); return; }")
|
||||
lines.add(" if (ret != 0) {")
|
||||
lines.add(
|
||||
@ -402,6 +406,12 @@ proc generateCAbiLibHeader*(
|
||||
lines.add("#define NIMFFI_RET_OK 0")
|
||||
lines.add("#define NIMFFI_RET_ERR 1")
|
||||
lines.add("#define NIMFFI_RET_MISSING_CALLBACK 2")
|
||||
lines.add("/* Non-terminal: the request is still running. Fires every ~5s with `msg`")
|
||||
lines.add(
|
||||
" carrying the elapsed milliseconds as decimal text; always followed by a"
|
||||
)
|
||||
lines.add(" terminal RET_OK/RET_ERR. Ignore it unless you want progress. */")
|
||||
lines.add("#define NIMFFI_RET_STALE_WARN 3")
|
||||
lines.add("")
|
||||
lines.add("/* Flat wire structs — the C ABI. Strings are borrowed, NUL-terminated")
|
||||
lines.add(" `const char*` valid only for the duration of the call they cross. */")
|
||||
|
||||
@ -80,35 +80,9 @@ proc abiCodegenImplemented*(fmt: ABIFormat): bool =
|
||||
|
||||
proc overrideKey*(override: string): string =
|
||||
## Lowercased key of a `key = value` pragma override (the text before `=`),
|
||||
## used to route it to its parser. `"timeout = 30000"` → `"timeout"`.
|
||||
## used to route it to its parser. `"abi = c"` → `"abi"`.
|
||||
override.split('=')[0].strip().toLowerAscii()
|
||||
|
||||
proc parseTimeoutSpec*(override: string): tuple[ok: bool, ms: int, err: string] =
|
||||
## Parse a `"timeout = <milliseconds>"` override (whitespace/case tolerant).
|
||||
## The value must be a positive integer number of milliseconds. On bad
|
||||
## grammar or value, returns `ok = false` with a human-readable `err`.
|
||||
let parts = override.split('=')
|
||||
if parts.len != 2 or overrideKey(override) != "timeout":
|
||||
return (
|
||||
false,
|
||||
0,
|
||||
"invalid timeout override: '" & override & "'; expected `timeout = <ms>`",
|
||||
)
|
||||
let raw = parts[1].strip()
|
||||
let ms =
|
||||
try:
|
||||
parseInt(raw)
|
||||
except ValueError:
|
||||
return (
|
||||
false,
|
||||
0,
|
||||
"invalid timeout value: '" & raw &
|
||||
"'; expected a positive integer of milliseconds",
|
||||
)
|
||||
if ms <= 0:
|
||||
return (false, 0, "timeout must be a positive number of milliseconds, got: " & raw)
|
||||
(true, ms, "")
|
||||
|
||||
proc parseABIFormatName*(name: string): tuple[ok: bool, fmt: ABIFormat] =
|
||||
## Bare format name (`"c"`/`"cbor"`, case-insensitive) → `ABIFormat`;
|
||||
## `ok` is false otherwise.
|
||||
|
||||
@ -368,16 +368,32 @@ proc generateApiRs*(
|
||||
lines.add(" } else {")
|
||||
lines.add(" slice::from_raw_parts(msg as *const u8, len).to_vec()")
|
||||
lines.add(" };")
|
||||
lines.add(" if ret == 0 { Ok(bytes) }")
|
||||
lines.add(" if ret == NIMFFI_RET_OK { Ok(bytes) }")
|
||||
lines.add(" else { Err(String::from_utf8_lossy(&bytes).into_owned()) }")
|
||||
lines.add("}")
|
||||
lines.add("")
|
||||
lines.add("// nim-ffi result-callback status codes (mirror ffi/ffi_types.nim).")
|
||||
lines.add("const NIMFFI_RET_OK: c_int = 0;")
|
||||
lines.add("const NIMFFI_RET_MISSING_CALLBACK: c_int = 2;")
|
||||
lines.add("const NIMFFI_RET_STALE_WARN: c_int = 3;")
|
||||
lines.add("")
|
||||
lines.add("unsafe extern \"C\" fn on_result(")
|
||||
lines.add(" ret: c_int,")
|
||||
lines.add(" msg: *const c_char,")
|
||||
lines.add(" len: usize,")
|
||||
lines.add(" user_data: *mut c_void,")
|
||||
lines.add(") {")
|
||||
lines.add(
|
||||
" // NIMFFI_RET_STALE_WARN (3) is a non-terminal progress ping: the request"
|
||||
)
|
||||
lines.add(
|
||||
" // is still running. This wrapper only delivers the final result, so ignore"
|
||||
)
|
||||
lines.add(
|
||||
" // it WITHOUT reclaiming the box — a terminal callback still owns the Sender."
|
||||
)
|
||||
lines.add(" if ret == NIMFFI_RET_STALE_WARN { return; }")
|
||||
lines.add("")
|
||||
lines.add(" // Take ownership of the boxed Sender — dropping it at end of scope")
|
||||
lines.add(" // releases the only outstanding handle.")
|
||||
lines.add(" let tx = Box::from_raw(user_data as *mut FFISender);")
|
||||
@ -419,7 +435,7 @@ proc generateApiRs*(
|
||||
lines.add(" let (tx, rx) = flume::bounded::<FFIResult>(1);")
|
||||
lines.add(" let raw = Box::into_raw(Box::new(tx)) as *mut c_void;")
|
||||
lines.add(" let ret = f(on_result, raw);")
|
||||
lines.add(" if ret == 2 {")
|
||||
lines.add(" if ret == NIMFFI_RET_MISSING_CALLBACK {")
|
||||
lines.add(" // Callback will never fire; reclaim the box to avoid a leak.")
|
||||
lines.add(" drop(unsafe { Box::from_raw(raw as *mut FFISender) });")
|
||||
lines.add(" return Err(\"RET_MISSING_CALLBACK (internal error)\".into());")
|
||||
@ -442,7 +458,7 @@ proc generateApiRs*(
|
||||
lines.add(" let (tx, rx) = flume::bounded::<FFIResult>(1);")
|
||||
lines.add(" let raw = Box::into_raw(Box::new(tx)) as *mut c_void;")
|
||||
lines.add(" let ret = f(on_result, raw);")
|
||||
lines.add(" if ret == 2 {")
|
||||
lines.add(" if ret == NIMFFI_RET_MISSING_CALLBACK {")
|
||||
lines.add(" drop(unsafe { Box::from_raw(raw as *mut FFISender) });")
|
||||
lines.add(" return Err(\"RET_MISSING_CALLBACK (internal error)\".into());")
|
||||
lines.add(" }")
|
||||
|
||||
@ -20,10 +20,17 @@ typedef void (*FFICallback)(int ret, const char* msg, size_t len, void* user_dat
|
||||
* value handed to a result callback's `err_code` (or returned by a submit call)
|
||||
* is a failure. NIMFFI_RET_MISSING_CALLBACK is a special case from the Nim
|
||||
* dispatcher: the callback will never fire, so the request path must report the
|
||||
* failure itself. */
|
||||
* failure itself.
|
||||
*
|
||||
* NIMFFI_RET_STALE_WARN is the one NON-terminal code: nim-ffi delivers it every
|
||||
* ~5s while a handler is still running (with `msg`/`len` carrying the elapsed
|
||||
* milliseconds as decimal text), then still ends with a terminal RET_OK/RET_ERR.
|
||||
* A caller that only wants the final answer must ignore it, not treat it as an
|
||||
* error. */
|
||||
#define NIMFFI_RET_OK 0
|
||||
#define NIMFFI_RET_ERROR 1
|
||||
#define NIMFFI_RET_MISSING_CALLBACK 2
|
||||
#define NIMFFI_RET_STALE_WARN 3
|
||||
|
||||
/* ── leaf encoders ─────────────────────────────────────────────────────── */
|
||||
static inline CborError nimffi_enc_bool(CborEncoder* e, const bool* v) {
|
||||
|
||||
@ -28,3 +28,13 @@
|
||||
extern "C" {
|
||||
#include <tinycbor/cbor.h>
|
||||
}
|
||||
|
||||
// nim-ffi result-callback status codes (mirror ffi/ffi_types.nim and the C
|
||||
// header). Guarded so a translation unit that also pulls in the C header keeps
|
||||
// a single definition.
|
||||
#ifndef NIMFFI_RET_OK
|
||||
#define NIMFFI_RET_OK 0
|
||||
#define NIMFFI_RET_ERR 1
|
||||
#define NIMFFI_RET_MISSING_CALLBACK 2
|
||||
#define NIMFFI_RET_STALE_WARN 3
|
||||
#endif
|
||||
|
||||
@ -17,6 +17,12 @@ struct FFICallState_ {
|
||||
};
|
||||
|
||||
inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) {
|
||||
// NIMFFI_RET_STALE_WARN (3) is a non-terminal progress ping: the request is
|
||||
// still running. This blocking wrapper only reports the final result, so
|
||||
// ignore it WITHOUT touching `ud` — a terminal callback still owns the
|
||||
// shared handle and will free it.
|
||||
if (ret == NIMFFI_RET_STALE_WARN) return;
|
||||
|
||||
// ffi_call_ heap-allocated a shared_ptr and passed its address as ud;
|
||||
// take ownership here so it's freed on every exit path.
|
||||
std::unique_ptr<std::shared_ptr<FFICallState_>> handle(
|
||||
@ -24,7 +30,7 @@ inline void ffi_cb_(int ret, const char* msg, size_t len, void* ud) {
|
||||
FFICallState_& s = **handle;
|
||||
|
||||
std::lock_guard<std::mutex> lock(s.mtx);
|
||||
s.ok = (ret == 0);
|
||||
s.ok = (ret == NIMFFI_RET_OK);
|
||||
if (msg && len > 0) {
|
||||
const auto* p = reinterpret_cast<const std::uint8_t*>(msg);
|
||||
if (s.ok) s.bytes.assign(p, p + len);
|
||||
@ -41,7 +47,7 @@ inline Result<std::vector<std::uint8_t>> ffi_call_(
|
||||
auto state = std::make_shared<FFICallState_>();
|
||||
auto* cb_ref = new std::shared_ptr<FFICallState_>(state);
|
||||
const int ret = f(ffi_cb_, cb_ref);
|
||||
if (ret == 2) {
|
||||
if (ret == NIMFFI_RET_MISSING_CALLBACK) {
|
||||
delete cb_ref;
|
||||
return Result<Bytes>::err("RET_MISSING_CALLBACK (internal error)");
|
||||
}
|
||||
|
||||
@ -42,12 +42,9 @@ type FFIContext*[T] = object
|
||||
# keeps the event thread draining until then so teardown-emitted events land
|
||||
running: Atomic[bool] # To control when the threads are running
|
||||
registeredRequests: ptr Table[cstring, FFIRequestProc]
|
||||
requestTimeouts: ptr Table[cstring, int]
|
||||
# Per-proc timeout overrides (ms). Points at the compile-time-filled global,
|
||||
# like registeredRequests, so the FFI thread reads it GC-safely via ctx.
|
||||
defaultRequestTimeout*: Duration
|
||||
# Per-handler deadline unless a `{.ffi: "timeout = <ms>".}` override raises
|
||||
# it; `InfiniteDuration` opts out. See processRequest for the trip behavior.
|
||||
staleWarnInterval*: Duration
|
||||
# RET_STALE_WARN cadence. An internal seam (tests tune it) — deliberately
|
||||
# not exposed to the ffi dev, and there is no per-proc override.
|
||||
|
||||
var onFFIThread* {.threadvar.}: bool
|
||||
# Re-entrant dispatch guard for `sendRequestToFFIThread`.
|
||||
@ -58,8 +55,12 @@ const
|
||||
EventThreadTickInterval* = 1.seconds
|
||||
FFIHeartbeatStartDelay* = 10.seconds # grace window for library startup
|
||||
FFIHeartbeatStaleThreshold* = 1.seconds
|
||||
DefaultRequestTimeout* = 5.seconds
|
||||
# Finite fallback (issue #93) so a wedged handler can't hang a caller forever.
|
||||
|
||||
const StaleWarnIntervalMs* {.intdefine: "ffiStaleWarnIntervalMs".} = 5000
|
||||
## `RET_STALE_WARN` cadence, fired without limit — nim-ffi never times a
|
||||
## handler out. 5s mirrors Android's ANR input timeout. Override with
|
||||
## `-d:ffiStaleWarnIntervalMs=<ms>`.
|
||||
const StaleWarnInterval* = StaleWarnIntervalMs.milliseconds
|
||||
|
||||
type FFITeardownProc*[T] = proc(lib: ptr T): Future[void] {.async.}
|
||||
|
||||
@ -130,7 +131,7 @@ proc initContextResources*[T](ctx: ptr FFIContext[T]): Result[void, string] =
|
||||
ctx.ffiHeartbeat.store(0)
|
||||
ctx.eventQueueStuck.store(false)
|
||||
ctx.ffiThreadExited.store(false)
|
||||
ctx.defaultRequestTimeout = DefaultRequestTimeout
|
||||
ctx.staleWarnInterval = StaleWarnInterval
|
||||
|
||||
var success = false
|
||||
defer:
|
||||
@ -146,7 +147,6 @@ proc initContextResources*[T](ctx: ptr FFIContext[T]): Result[void, string] =
|
||||
newSignalOrErr(ctx.eventThreadExitSignal, "eventThreadExitSignal")
|
||||
|
||||
ctx.registeredRequests = addr ffi_types.registeredRequests
|
||||
ctx.requestTimeouts = addr ffi_types.requestTimeoutsMs
|
||||
|
||||
ctx.running.store(true)
|
||||
|
||||
|
||||
@ -44,46 +44,36 @@ proc sendRequestToFFIThread*(
|
||||
|
||||
ok()
|
||||
|
||||
func resolveRequestTimeout[T](reqIdCs: cstring, ctx: ptr FFIContext[T]): Duration =
|
||||
## Per-proc `{.ffi: "timeout = <ms>".}` override if one was registered for this
|
||||
## request type, otherwise the context-wide default.
|
||||
let ms = ctx[].requestTimeouts[].getOrDefault(reqIdCs, 0)
|
||||
if ms > 0: ms.milliseconds else: ctx.defaultRequestTimeout
|
||||
|
||||
proc reportTimeoutIfTripped(
|
||||
proc awaitWithStaleWarnings(
|
||||
retFut: Future[Result[seq[byte], string]],
|
||||
request: ptr FFIThreadRequest,
|
||||
deadline: Duration,
|
||||
interval: Duration,
|
||||
reqId: string,
|
||||
) {.async.} =
|
||||
## Waits for the handler or its deadline, whichever comes first. On a trip we
|
||||
## deliberately do NOT cancel the handler: a hard-cancel mid-call into the
|
||||
## underlying library (Waku/libp2p) can leave it partially applied, so we
|
||||
## unblock the caller with a timeout err now and let the handler run to
|
||||
## completion. `fireCallback`'s once-only guard keeps the two paths from
|
||||
## answering twice.
|
||||
if deadline == InfiniteDuration:
|
||||
return
|
||||
# Handlers that already completed (e.g. a sync body) skip the timer entirely,
|
||||
# keeping the per-request cost off the fast path.
|
||||
if retFut.finished():
|
||||
return
|
||||
let timer = sleepAsync(deadline)
|
||||
# `race` returns the first to finish WITHOUT cancelling the loser, so the
|
||||
# handler keeps running when the timer wins.
|
||||
discard await race(retFut, timer)
|
||||
if not timer.finished():
|
||||
await timer.cancelAndWait()
|
||||
if retFut.finished():
|
||||
return
|
||||
warn "ffi request timed out; caller unblocked, handler left running",
|
||||
reqId = reqId, timeoutMs = deadline.milliseconds
|
||||
fireCallback(
|
||||
Result[seq[byte], string].err(
|
||||
"ffi request timed out after " & $deadline.milliseconds & "ms"
|
||||
),
|
||||
request,
|
||||
)
|
||||
): Future[Result[seq[byte], string]] {.async.} =
|
||||
## Pings the caller with RET_STALE_WARN every `interval` while the handler
|
||||
## runs, then returns its real result. Never cancels it: a hard-cancel mid-call
|
||||
## into the underlying library (Waku/libp2p) can leave it partially applied, so
|
||||
## the caller is kept informed and decides for itself. The timer lives entirely
|
||||
## in this frame, so nothing references the request once the handler resolves.
|
||||
let intervalMs = interval.milliseconds
|
||||
if intervalMs <= 0:
|
||||
# A non-positive / infinite interval opts out of progress pings entirely.
|
||||
return await retFut
|
||||
var elapsed = 0'i64
|
||||
while not retFut.finished():
|
||||
let timer = sleepAsync(interval)
|
||||
# `race` returns the first to finish WITHOUT cancelling the loser, so the
|
||||
# handler keeps running when the timer wins.
|
||||
discard await race(retFut, timer)
|
||||
if retFut.finished():
|
||||
if not timer.finished():
|
||||
await timer.cancelAndWait()
|
||||
break
|
||||
elapsed += intervalMs
|
||||
warn "ffi request still in flight; caller notified via RET_STALE_WARN",
|
||||
reqId = reqId, elapsedMs = elapsed
|
||||
fireStaleWarn(request, elapsed)
|
||||
return await retFut
|
||||
|
||||
proc processRequest[T](
|
||||
request: ptr FFIThreadRequest, ctx: ptr FFIContext[T]
|
||||
@ -100,16 +90,12 @@ proc processRequest[T](
|
||||
else:
|
||||
ctx[].registeredRequests[][reqIdCs](cast[pointer](request), ctx)
|
||||
|
||||
# CatchableError covers CancelledError from the shutdown drain; handleRes must
|
||||
# still run, so the timeout race and the handler await share one try — a cancel
|
||||
# mid-race must not skip the response-and-free below.
|
||||
# CatchableError covers CancelledError from the shutdown drain. The warn loop
|
||||
# and the handler share one try so that a cancel mid-loop still reaches the
|
||||
# response-and-free below.
|
||||
let res =
|
||||
try:
|
||||
# May answer the caller early with a timeout err; the handler keeps running.
|
||||
await reportTimeoutIfTripped(
|
||||
retFut, request, resolveRequestTimeout(reqIdCs, ctx), reqId
|
||||
)
|
||||
await retFut
|
||||
await awaitWithStaleWarnings(retFut, request, ctx.staleWarnInterval, reqId)
|
||||
except CatchableError as e:
|
||||
Result[seq[byte], string].err(
|
||||
"Error in processRequest for " & reqId & ": " & e.msg
|
||||
|
||||
@ -251,9 +251,26 @@ proc fireCallback*(res: Result[seq[byte], string], request: ptr FFIThreadRequest
|
||||
RET_OK, cast[ptr cchar](addr sentinel), 1.csize_t, request[].userData
|
||||
)
|
||||
|
||||
proc fireStaleWarn*(request: ptr FFIThreadRequest, elapsedMs: int64) =
|
||||
## Tells the caller its request is still in flight after `elapsedMs` (sent as
|
||||
## decimal UTF-8). Unlike `fireCallback` it deliberately leaves `responded`
|
||||
## unset and may fire many times — the terminal RET_OK/RET_ERR is still owed.
|
||||
## Runs on the FFI thread, so reading `responded` needs no synchronization.
|
||||
if request[].responded:
|
||||
return
|
||||
foreignThreadGc:
|
||||
let msg = $elapsedMs
|
||||
request[].callback(
|
||||
RET_STALE_WARN,
|
||||
cast[ptr cchar](unsafeAddr msg[0]),
|
||||
cast[csize_t](msg.len),
|
||||
request[].userData,
|
||||
)
|
||||
|
||||
proc handleRes*(res: Result[seq[byte], string], request: ptr FFIThreadRequest) =
|
||||
## Terminal step of every request: delivers the response (unless a timeout
|
||||
## already did) and frees the request exactly once.
|
||||
## Terminal step of every request: delivers the response and frees the request
|
||||
## exactly once. The `responded` guard in `fireStaleWarn` keeps this answer
|
||||
## last, after any progress pings.
|
||||
defer:
|
||||
deleteRequest(request)
|
||||
fireCallback(res, request)
|
||||
|
||||
@ -7,10 +7,20 @@ import chronos
|
||||
type FFICallBack* = proc(
|
||||
callerRet: cint, msg: ptr cchar, len: csize_t, userData: pointer
|
||||
) {.cdecl, gcsafe, raises: [].}
|
||||
## Result-delivery callback. `callerRet` is one of the `RET_*` codes below:
|
||||
## `RET_OK`/`RET_ERR` fire exactly once and end the request, `RET_STALE_WARN`
|
||||
## may fire repeatedly before them and should be ignored unless progress
|
||||
## matters.
|
||||
|
||||
const RET_OK*: cint = 0
|
||||
const RET_ERR*: cint = 1
|
||||
const RET_MISSING_CALLBACK*: cint = 2
|
||||
const RET_STALE_WARN*: cint = 3
|
||||
## Non-terminal: the request is still in flight. Fires every
|
||||
## `StaleWarnInterval` (default 5s) while the handler runs, `msg` carrying the
|
||||
## elapsed milliseconds as decimal ASCII, and is always followed by a terminal
|
||||
## code — nim-ffi never times a handler out, so the caller decides whether to
|
||||
## keep waiting.
|
||||
|
||||
### End of exported types
|
||||
################################################################################
|
||||
@ -37,11 +47,5 @@ template foreignThreadGc*(body: untyped) =
|
||||
## The value is a proc that handles the request asynchronously.
|
||||
var registeredRequests*: Table[cstring, FFIRequestProc]
|
||||
|
||||
## Per-request handler-timeout overrides in milliseconds, keyed by the same Req
|
||||
## type name as `registeredRequests`. Populated at compile time from a
|
||||
## `{.ffi: "timeout = <ms>".}` spec; an absent key means "use the context's
|
||||
## `defaultRequestTimeout`". Like `registeredRequests`, never mutated at run time.
|
||||
var requestTimeoutsMs*: Table[cstring, int]
|
||||
|
||||
### End of FFI utils
|
||||
################################################################################
|
||||
|
||||
@ -686,6 +686,11 @@ proc objectTrampBody(boxName, respType, respWire: NimNode): NimNode =
|
||||
let box = cast[ptr `boxName`](ud)
|
||||
if box.isNil():
|
||||
return
|
||||
if ret == RET_STALE_WARN:
|
||||
# Non-terminal progress signal: keep the box for the eventual terminal
|
||||
# reply and don't decode (there's no reply payload yet). Typed wrappers
|
||||
# don't surface it; the raw FFICallBack boundary does.
|
||||
return
|
||||
defer:
|
||||
freeBox(box)
|
||||
if box.fn.isNil():
|
||||
@ -719,6 +724,11 @@ proc stringTrampBody(boxName: NimNode): NimNode =
|
||||
let box = cast[ptr `boxName`](ud)
|
||||
if box.isNil():
|
||||
return
|
||||
if ret == RET_STALE_WARN:
|
||||
# Non-terminal progress signal: keep the box for the eventual terminal
|
||||
# reply and don't decode. Typed wrappers don't surface it; the raw
|
||||
# FFICallBack boundary does.
|
||||
return
|
||||
defer:
|
||||
freeBox(box)
|
||||
if box.fn.isNil():
|
||||
|
||||
@ -61,19 +61,14 @@ proc resolveABIFormat(abiSpecs: seq[NimNode]): ABIFormat {.compileTime.} =
|
||||
fmt = parsed.fmt
|
||||
fmt
|
||||
|
||||
proc resolveFFISpecs(
|
||||
specs: seq[NimNode]
|
||||
): tuple[abi: ABIFormat, timeoutMs: int] {.compileTime.} =
|
||||
## Resolve an annotation's `"abi = ..."` and `"timeout = ..."` string specs
|
||||
## (last of each wins), inheriting the library-default ABI when absent.
|
||||
## `timeoutMs == 0` means "no per-proc override" (use the context default).
|
||||
proc resolveFFISpecs(specs: seq[NimNode]): ABIFormat {.compileTime.} =
|
||||
## Resolve an annotation's `"abi = ..."` string specs (last wins), inheriting
|
||||
## the library-default ABI when absent.
|
||||
var abi = currentDefaultABIFormat
|
||||
var timeoutMs = 0
|
||||
for override in specs:
|
||||
if override.kind notin {nnkStrLit, nnkRStrLit, nnkTripleStrLit}:
|
||||
error(
|
||||
"FFI override must be a string literal like \"abi = c\" or " &
|
||||
"\"timeout = 30000\", got: " & override.repr
|
||||
"FFI override must be a string literal like \"abi = c\", got: " & override.repr
|
||||
)
|
||||
case overrideKey($override)
|
||||
of "abi":
|
||||
@ -81,30 +76,9 @@ proc resolveFFISpecs(
|
||||
if not parsed.ok:
|
||||
error(parsed.err)
|
||||
abi = parsed.fmt
|
||||
of "timeout":
|
||||
let parsed = parseTimeoutSpec($override)
|
||||
if not parsed.ok:
|
||||
error(parsed.err)
|
||||
timeoutMs = parsed.ms
|
||||
else:
|
||||
error(
|
||||
"unknown FFI override '" & $override &
|
||||
"'; expected `abi = ...` or `timeout = ...`"
|
||||
)
|
||||
(abi, timeoutMs)
|
||||
|
||||
proc registerRequestTimeout(
|
||||
reqTypeName: NimNode, timeoutMs: int
|
||||
): NimNode {.compileTime.} =
|
||||
## Top-level assignment that records a per-proc handler timeout at module init,
|
||||
## keyed by the same Req type name the dispatcher registry uses. Empty when no
|
||||
## override was given.
|
||||
if timeoutMs <= 0:
|
||||
return newStmtList()
|
||||
newAssignment(
|
||||
newTree(nnkBracketExpr, ident("requestTimeoutsMs"), newLit($reqTypeName)),
|
||||
newLit(timeoutMs),
|
||||
)
|
||||
error("unknown FFI override '" & $override & "'; expected `abi = ...`")
|
||||
abi
|
||||
|
||||
proc gateABIFormat(fmt: ABIFormat, where: string) {.compileTime.} =
|
||||
## Abort if the selected ABI's codegen isn't wired yet (only `Cbor` is), so a
|
||||
@ -678,7 +652,7 @@ macro ffiRaw*(args: varargs[untyped]): untyped =
|
||||
requireBeforeGenBindings("`.ffiRaw.`")
|
||||
requireLibraryDeclared("`.ffiRaw.`")
|
||||
let prc = args[^1]
|
||||
let (rawAbiFormat, rawTimeoutMs) = resolveFFISpecs(args[0 ..^ 2])
|
||||
let rawAbiFormat = resolveFFISpecs(args[0 ..^ 2])
|
||||
gateABIFormat(rawAbiFormat, "`.ffiRaw.` proc")
|
||||
|
||||
let procName = prc[0]
|
||||
@ -752,8 +726,7 @@ macro ffiRaw*(args: varargs[untyped]): untyped =
|
||||
registerReqFFI(`reqName`, `paramIdent`: `paramType`):
|
||||
`anonymousProcNode`
|
||||
|
||||
let stmts =
|
||||
newStmtList(registerReq, ffiProc, registerRequestTimeout(reqName, rawTimeoutMs))
|
||||
let stmts = newStmtList(registerReq, ffiProc)
|
||||
|
||||
when defined(ffiDumpMacros):
|
||||
echo stmts.repr
|
||||
@ -827,14 +800,12 @@ macro ffi*(args: varargs[untyped]): untyped =
|
||||
requireBeforeGenBindings("`.ffi.`")
|
||||
# Annotated node is the last vararg; leading args are `"abi = ..."` specs.
|
||||
let prc = args[^1]
|
||||
let (abiFormat, timeoutMs) = resolveFFISpecs(args[0 ..^ 2])
|
||||
let abiFormat = resolveFFISpecs(args[0 ..^ 2])
|
||||
|
||||
# A value type stands alone (no library required). Its `c` companion is
|
||||
# emitted later by `genBindings()`, since a type-pragma macro can only return
|
||||
# a TypeDef; `cbor` rides the generic overloads. Both abis are valid here.
|
||||
if prc.kind == nnkTypeDef:
|
||||
if timeoutMs > 0:
|
||||
error("`.ffi.` on a type takes no `timeout` override (it only applies to procs)")
|
||||
gateFFITypeABIFormat(abiFormat, "`.ffi.` type")
|
||||
var cleanTypeDef = prc.copyNimTree()
|
||||
if cleanTypeDef[0].kind == nnkPragmaExpr:
|
||||
@ -1096,13 +1067,9 @@ macro ffi*(args: varargs[untyped]): untyped =
|
||||
cExportName, libTypeName, reqTypeName, extraParamNames, extraParamTypes,
|
||||
resultRetType,
|
||||
)
|
||||
return newStmtList(
|
||||
helperProc, registerReq, registerRequestTimeout(reqTypeName, timeoutMs)
|
||||
)
|
||||
return newStmtList(helperProc, registerReq)
|
||||
|
||||
return newStmtList(
|
||||
helperProc, registerReq, ffiProc, registerRequestTimeout(reqTypeName, timeoutMs)
|
||||
)
|
||||
return newStmtList(helperProc, registerReq, ffiProc)
|
||||
|
||||
proc scalarPath(): NimNode =
|
||||
## The scalar fast path lives in `ffi_scalar`; here we only build the shared
|
||||
@ -1377,7 +1344,7 @@ macro ffiCtor*(args: varargs[untyped]): untyped =
|
||||
requireBeforeGenBindings("`.ffiCtor.`")
|
||||
requireLibraryDeclared("`.ffiCtor.`")
|
||||
let prc = args[^1]
|
||||
let (abiFormat, timeoutMs) = resolveFFISpecs(args[0 ..^ 2])
|
||||
let abiFormat = resolveFFISpecs(args[0 ..^ 2])
|
||||
gateABIFormat(abiFormat, "`.ffiCtor.` proc")
|
||||
|
||||
let procName = prc[0]
|
||||
@ -1551,25 +1518,10 @@ macro ffiCtor*(args: varargs[untyped]): untyped =
|
||||
# The flat-struct exported wrapper is emitted at genBindings() time (see
|
||||
# flushCAbiDispatch); the CBOR `ffiProc` is not.
|
||||
registerCAbiCtor(cExportName, libTypeName, reqTypeName, paramNames, paramTypes)
|
||||
newStmtList(
|
||||
typeDef,
|
||||
ffiNewReqProc,
|
||||
helperProc,
|
||||
processProc,
|
||||
addToReg,
|
||||
poolDecl,
|
||||
registerRequestTimeout(reqTypeName, timeoutMs),
|
||||
)
|
||||
newStmtList(typeDef, ffiNewReqProc, helperProc, processProc, addToReg, poolDecl)
|
||||
else:
|
||||
newStmtList(
|
||||
typeDef,
|
||||
ffiNewReqProc,
|
||||
helperProc,
|
||||
processProc,
|
||||
addToReg,
|
||||
poolDecl,
|
||||
ffiProc,
|
||||
registerRequestTimeout(reqTypeName, timeoutMs),
|
||||
typeDef, ffiNewReqProc, helperProc, processProc, addToReg, poolDecl, ffiProc
|
||||
)
|
||||
|
||||
when defined(ffiDumpMacros):
|
||||
|
||||
@ -38,13 +38,6 @@ proc abitest_echo*(
|
||||
): Future[Result[int, string]] {.ffi: "abi = cbor".} =
|
||||
return ok(n)
|
||||
|
||||
# Per-proc handler-timeout override (issue #93): parsed like the abi spec and
|
||||
# recorded in `requestTimeoutsMs`, keyed by the generated Req type name.
|
||||
proc abitest_slow*(
|
||||
lib: AbiLib, n: int
|
||||
): Future[Result[int, string]] {.ffi: "timeout = 30000".} =
|
||||
return ok(n)
|
||||
|
||||
# Event with an explicit ABI override passed after the wire name.
|
||||
proc abitest_pinged*(p: Pinged) {.ffiEvent("on_pinged", "abi = cbor").}
|
||||
|
||||
@ -98,30 +91,12 @@ suite "ABI format parsing":
|
||||
check parseAbiSpec("abi = bson").ok == false # unknown format
|
||||
check "bson" in parseAbiSpec("abi = bson").err
|
||||
|
||||
suite "handler-timeout spec parsing (issue #93)":
|
||||
suite "pragma override key parsing":
|
||||
test "overrideKey extracts the lowercased, trimmed key":
|
||||
check overrideKey("timeout = 30000") == "timeout"
|
||||
check overrideKey("abi = c") == "abi"
|
||||
check overrideKey(" ABI = c ") == "abi"
|
||||
check overrideKey("bare") == "bare"
|
||||
|
||||
test "parseTimeoutSpec accepts `timeout = <ms>`, flexible spacing":
|
||||
check parseTimeoutSpec("timeout = 30000") == (true, 30000, "")
|
||||
check parseTimeoutSpec("TIMEOUT=100").ms == 100
|
||||
check parseTimeoutSpec(" timeout = 5 ").ms == 5
|
||||
|
||||
test "parseTimeoutSpec rejects malformed specs and non-positive values":
|
||||
check parseTimeoutSpec("30000").ok == false # missing `timeout =`
|
||||
check parseTimeoutSpec("abi = c").ok == false # wrong key
|
||||
check parseTimeoutSpec("timeout = 1 = 2").ok == false # too many `=`
|
||||
check parseTimeoutSpec("timeout = abc").ok == false # not an integer
|
||||
check parseTimeoutSpec("timeout = 0").ok == false # must be positive
|
||||
check parseTimeoutSpec("timeout = -5").ok == false # must be positive
|
||||
|
||||
test "a `timeout` override is recorded; a plain proc has no entry":
|
||||
# Populated at module init from the annotations above.
|
||||
check requestTimeoutsMs["AbitestSlowReq".cstring] == 30000
|
||||
check not requestTimeoutsMs.hasKey("AbitestPingReq".cstring)
|
||||
|
||||
suite "ABI proc-dispatch readiness":
|
||||
test "both cbor and c proc-dispatch are wired":
|
||||
# This predicate is what the proc-form macros consult. Both ABIs now have a
|
||||
|
||||
@ -27,6 +27,10 @@ proc deinitCallbackData(d: var CallbackData) =
|
||||
proc testCallback(
|
||||
retCode: cint, msg: ptr cchar, len: csize_t, userData: pointer
|
||||
) {.cdecl, gcsafe, raises: [].} =
|
||||
# A progress ping is not an answer: waking waitCallback here would report a
|
||||
# non-terminal code as the result. Tests asserting on pings use staleCallback.
|
||||
if retCode == RET_STALE_WARN:
|
||||
return
|
||||
let d = cast[ptr CallbackData](userData)
|
||||
acquire(d[].lock)
|
||||
d[].retCode = retCode
|
||||
@ -228,12 +232,6 @@ suite "destroyFFIContext refc workaround":
|
||||
check false
|
||||
return
|
||||
|
||||
# This case stresses the refc destroy workaround, not the request timeout:
|
||||
# the 50k-allocation handler can outrun the finite default deadline on a slow
|
||||
# sanitizer/ARM runner, tripping a spurious timeout err. Opt out so the check
|
||||
# below observes the handler's real result.
|
||||
ctx.defaultRequestTimeout = InfiniteDuration
|
||||
|
||||
var d: CallbackData
|
||||
initCallbackData(d)
|
||||
defer:
|
||||
@ -619,25 +617,18 @@ suite "reentrancy guard (PR #23 review, item 6)":
|
||||
check nestedMsg.startsWith("err:")
|
||||
check "reentrant ffi call" in nestedMsg
|
||||
|
||||
# Per-proc handler timeout (issue #93): a `{.ffi: "timeout = <ms>".}` override
|
||||
# bounds how long a handler may run before the caller is unblocked with an err.
|
||||
# The handler is NOT cancelled — it keeps running — so the callback must still
|
||||
# fire exactly once.
|
||||
# Non-terminal RET_STALE_WARN progress signal: while a handler runs, the caller
|
||||
# is pinged every `ctx.staleWarnInterval` with the elapsed time, then still gets
|
||||
# exactly one terminal RET_OK/RET_ERR. nim-ffi never times the handler out.
|
||||
|
||||
type TimeoutConfig {.ffi.} = object
|
||||
type StaleConfig {.ffi.} = object
|
||||
dummy: int
|
||||
|
||||
proc testlib_slow_timeout*(
|
||||
lib: SimpleLib, cfg: TimeoutConfig
|
||||
): Future[Result[string, string]] {.ffi: "timeout = 100".} =
|
||||
await sleepAsync(500.milliseconds)
|
||||
return ok("slow-timeout-done")
|
||||
|
||||
proc testlib_under_deadline*(
|
||||
lib: SimpleLib, cfg: TimeoutConfig
|
||||
): Future[Result[string, string]] {.ffi: "timeout = 1000".} =
|
||||
await sleepAsync(50.milliseconds)
|
||||
return ok("under-deadline-done")
|
||||
proc testlib_slow_stale*(
|
||||
lib: SimpleLib, cfg: StaleConfig
|
||||
): Future[Result[string, string]] {.ffi.} =
|
||||
await sleepAsync(350.milliseconds)
|
||||
return ok("slow-stale-done")
|
||||
|
||||
proc createSimpleCtx(): ptr FFIContext[SimpleLib] =
|
||||
## Spins up a SimpleLib context via the ctor and returns it (nil on failure).
|
||||
@ -658,51 +649,86 @@ proc createSimpleCtx(): ptr FFIContext[SimpleLib] =
|
||||
return nil
|
||||
cast[ptr FFIContext[SimpleLib]](ctxAddr)
|
||||
|
||||
suite "per-proc request timeout (issue #93)":
|
||||
test "handler past its deadline yields a timeout err, fired exactly once":
|
||||
## Callback that keeps the non-terminal stale pings apart from the one terminal
|
||||
## answer, so a test can assert on both.
|
||||
type StaleData = object
|
||||
lock: Lock
|
||||
cond: Cond
|
||||
staleCount: int
|
||||
lastElapsed: string
|
||||
terminalDone: bool
|
||||
terminalRet: cint
|
||||
terminalBytes: seq[byte]
|
||||
|
||||
proc initStaleData(d: var StaleData) =
|
||||
d.lock.initLock()
|
||||
d.cond.initCond()
|
||||
|
||||
proc deinitStaleData(d: var StaleData) =
|
||||
d.cond.deinitCond()
|
||||
d.lock.deinitLock()
|
||||
|
||||
proc staleCallback(
|
||||
retCode: cint, msg: ptr cchar, len: csize_t, userData: pointer
|
||||
) {.cdecl, gcsafe, raises: [].} =
|
||||
let d = cast[ptr StaleData](userData)
|
||||
let n = int(len)
|
||||
acquire(d[].lock)
|
||||
if retCode == RET_STALE_WARN:
|
||||
var s = newString(n)
|
||||
if n > 0 and not msg.isNil:
|
||||
copyMem(addr s[0], msg, n)
|
||||
inc d[].staleCount
|
||||
d[].lastElapsed = s
|
||||
else:
|
||||
var b = newSeq[byte](n)
|
||||
if n > 0 and not msg.isNil:
|
||||
copyMem(addr b[0], msg, n)
|
||||
d[].terminalRet = retCode
|
||||
d[].terminalBytes = b
|
||||
d[].terminalDone = true
|
||||
signal(d[].cond)
|
||||
release(d[].lock)
|
||||
|
||||
proc waitTerminal(d: var StaleData) =
|
||||
acquire(d.lock)
|
||||
while not d.terminalDone:
|
||||
wait(d.cond, d.lock)
|
||||
release(d.lock)
|
||||
|
||||
suite "non-terminal RET_STALE_WARN progress signal":
|
||||
test "a slow handler pings the caller, then delivers one terminal RET_OK":
|
||||
let ctx = createSimpleCtx()
|
||||
check not ctx.isNil()
|
||||
defer:
|
||||
check SimpleLibFFIPool.destroyFFIContext(ctx).isOk()
|
||||
|
||||
var d: CallbackData
|
||||
initCallbackData(d)
|
||||
defer:
|
||||
deinitCallbackData(d)
|
||||
# Tune the cadence down so the 350 ms handler trips several pings quickly.
|
||||
ctx.staleWarnInterval = 80.milliseconds
|
||||
|
||||
var reqBytes = cborEncode(TestlibSlowTimeoutReq(cfg: TimeoutConfig(dummy: 0)))
|
||||
let ret = testlib_slow_timeout(
|
||||
ctx, testCallback, addr d, encodedPtr(reqBytes), reqBytes.len.csize_t
|
||||
var d: StaleData
|
||||
initStaleData(d)
|
||||
defer:
|
||||
deinitStaleData(d)
|
||||
|
||||
var reqBytes = cborEncode(TestlibSlowStaleReq(cfg: StaleConfig(dummy: 0)))
|
||||
let ret = testlib_slow_stale(
|
||||
ctx, staleCallback, addr d, encodedPtr(reqBytes), reqBytes.len.csize_t
|
||||
)
|
||||
check ret == RET_OK
|
||||
|
||||
waitCallback(d)
|
||||
check d.retCode == RET_ERR
|
||||
check "timed out" in callbackErr(d)
|
||||
waitTerminal(d)
|
||||
|
||||
# The handler (500 ms) is still running past the 100 ms deadline; once it
|
||||
# finishes it must NOT deliver a second callback.
|
||||
os.sleep(700)
|
||||
check d.callCount == 1
|
||||
# A 350 ms handler at an 80 ms cadence trips at least a couple of pings, and
|
||||
# the k-th ping reports exactly k*80 ms of elapsed time.
|
||||
check d.staleCount >= 2
|
||||
check parseInt(d.lastElapsed) == d.staleCount * 80
|
||||
|
||||
test "handler finishing under its deadline returns normally":
|
||||
let ctx = createSimpleCtx()
|
||||
check not ctx.isNil()
|
||||
defer:
|
||||
check SimpleLibFFIPool.destroyFFIContext(ctx).isOk()
|
||||
# Exactly one terminal answer carrying the handler's real result.
|
||||
check d.terminalRet == RET_OK
|
||||
check cborDecode(d.terminalBytes, string).value == "slow-stale-done"
|
||||
|
||||
var d: CallbackData
|
||||
initCallbackData(d)
|
||||
defer:
|
||||
deinitCallbackData(d)
|
||||
|
||||
var reqBytes = cborEncode(TestlibUnderDeadlineReq(cfg: TimeoutConfig(dummy: 0)))
|
||||
let ret = testlib_under_deadline(
|
||||
ctx, testCallback, addr d, encodedPtr(reqBytes), reqBytes.len.csize_t
|
||||
)
|
||||
check ret == RET_OK
|
||||
|
||||
waitCallback(d)
|
||||
check d.retCode == RET_OK
|
||||
check cborDecode(callbackBytes(d), string).value == "under-deadline-done"
|
||||
check d.callCount == 1
|
||||
# Nothing trails the terminal callback.
|
||||
let staleAtTerminal = d.staleCount
|
||||
os.sleep(200)
|
||||
check d.staleCount == staleAtTerminal
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user