Skip to content

Make plugin API cleanup functions noexcept - #13590

Draft
bryancall wants to merge 2 commits into
apache:masterfrom
bryancall:coverity-noexcept-plugin-api
Draft

Make plugin API cleanup functions noexcept#13590
bryancall wants to merge 2 commits into
apache:masterfrom
bryancall:coverity-noexcept-plugin-api

Conversation

@bryancall

@bryancall bryancall commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Exceptions have never been a supported error channel across the plugin API: API functions report failure through return values, and no header or document has ever promised that a TS API call throws anything a plugin could catch. That contract was never written down, though, so any exception escaping from the core into plugin code is both undiagnosable and invisible to static analysis.

It matters most in destructors. Plugin destructors routinely call the API cleanup functions, destructors are implicitly noexcept, and so an exception escaping one of those calls terminates the process with no attribution at all.

What this does

  • Annotates 28 API functions noexcept in ts.h, the cleanup/teardown family that destructors call: TSContDestroy, TSHandleMLocRelease, TSIOBuffer*/TSMBufferDestroy, TSHttpHdrDestroy, TSMimeHdrDestroy, TSVConn{Close,Abort,Shutdown}, TSActionCancel, TSCacheKeyDestroy, TSSslContextDestroy, TSFetchDestroy, TSTextLogObject*, the TSMutex* family, TSThread{Wait,Destroy}, TSContData{Get,Set}, and TSError).
  • Converts each definition to a function-try-block whose handler calls ink_abort("exception escaped %s", __func__). An exception that would previously have escaped now produces a named, attributable abort instead of a bare std::terminate inside a plugin destructor.
  • Annotates ts::do_abort [[noreturn]] noexcept and contains formatting failures inside it, and does the same for ts::shared_mutex::_call_fatal. Both build their message with swoc::bwprint/Strerror, which allocate, so the abort path could itself throw bad_alloc before reaching the abort, from inside ~write_guard/~read_guard.
  • Updates the matching prototypes in the developer guide (22 pages) and states the no-exceptions contract in the plugin getting-started chapter.

Overhead

None on the happy path, as expected for table-driven exception handling. Comparing generated code for a representative wrapped function against the same function unwrapped, the instruction sequences are identical except that the final call can no longer be a tail call. Measured on InkAPI.cc built before and after with identical flags: __text grew 896 bytes (+0.66%, about 45 bytes per wrapped function) and __gcc_except_tab grew 500 bytes of read-only data that is never touched unless an exception unwinds.

In the other direction this is a small optimization enabler: callers that can see noexcept no longer need their own landing pads around these calls, so plugin cleanup code should get marginally smaller.

Coverity

This should resolve 83 Coverity UNCAUGHT_EXCEPT defects.

The checker flags this pattern because it has to assume any call in a destructor can throw. I arrived at that number by parsing the body of every destructor it flags and classifying what each one calls: 80 whose only calls are functions this PR makes noexcept, plus 3 ts::shared_mutex guard destructors covered by the _call_fatal fix. By area that is 61 in plugins, 13 in tscpp/api, 5 in core, 2 in examples, and 2 in test code.

Treat 83 as a projection rather than a measurement. It assumes the checker credits a noexcept callee and stops reporting the caller, which is the documented behavior but is not something I can confirm without a scan of the merged tree. The first analysis run after this lands will give the real figure, and I will follow up with it.

Destructors whose throw path is a member subobject rather than an API call are not affected by this change and need their own root cause, as do the CLI tool entry points. Those are follow-up work.

ABI

Adding noexcept does not change the ABI. In the Itanium C++ ABI an exception specification is part of a function type but is not encoded in the mangled name of a function declaration, and it does not affect the calling convention.

I verified this rather than assuming it. Building src/api/InkAPI.cc before and after with identical flags, both objects export 748 text symbols with zero name differences: TSContDestroy is _Z13TSContDestroyP10tsapi_cont in both, TSError is _Z7TSErrorPKcz in both. As an end-to-end check, a caller compiled against the old declaration and linked against the new noexcept definition resolves to the same symbol, links, and runs. So an existing plugin binary keeps working against a rebuilt libtsapi and vice versa.

Function pointers are also unaffected: a noexcept function converts implicitly to a plain function-pointer type, so void (*p)(TSCont) = TSContDestroy; still compiles. Mangling would only shift if a parameter or template argument were spelled as a noexcept function type, which nothing in the API does. Nothing in the tree takes the address of any annotated function.

The compatibility cost is at the source level, not the binary level, and is described below.

Notes for reviewers

  • Behavior change: a plugin that wrapped a TS API call in a catch expecting to catch a core-internal exception would now see an abort. Exceptions were never a supported error channel here, so this standardizes existing practice rather than removing a documented one. It is a real change, though, and worth a release note.
  • Source compatibility: noexcept has been part of the function type since C++17, so any translation unit that includes ts.h and separately declares one of these functions without noexcept becomes ill-formed. This applies equally to a plugin built as C++17 or C++20. Seven in-tree test stubs that redefine API functions needed the annotation added; out-of-tree plugins doing the same will need the same one-word change. The stub edits are mechanical and have to land in the same commit or the build breaks, which is why they are here rather than split out.
  • Selection rule: the set is "the cleanup/teardown entry points a destructor can reasonably call," derived by parsing what the flagged destructors actually call, then extended to the obvious siblings (TSMutexDestroy, TSMutexLockTry, TSMimeHdrDestroy, TSSslContextDestroy) so the boundary is not an artifact of a scanner hit list. Widening it to the whole API surface is a reasonable follow-up if reviewers prefer that; it did not seem like the right first step.

Exceptions have never been a supported error channel across the plugin
API; failures are reported through return values. This writes that
contract down for the functions plugins commonly call during cleanup
(destroy/free/release/close/cancel, the mutex family, and TSError):

- Annotate 28 API functions noexcept in ts.h and their definitions.
- Convert each definition to a function-try-block so an internal
  exception that would have escaped now aborts with a message naming
  the function, instead of terminating with no context inside a plugin
  destructor (destructors are implicitly noexcept).
- Annotate ts::do_abort [[noreturn]] noexcept and contain formatting
  failures inside it and inside ts::shared_mutex::_call_fatal, whose
  message formatting allocates and could itself throw before the abort.
- Update matching prototypes in the developer guide and state the
  no-exceptions contract in the plugin getting-started chapter.
@bryancall bryancall self-assigned this Aug 26, 2026
@bryancall bryancall added this to the 11.0.0 milestone Aug 26, 2026
@bryancall

Copy link
Copy Markdown
Contributor Author

[approve ci autest 2]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant