You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "bryancall (via GitHub)" <gi...@apache.org> on 2026/08/26 18:48:32 UTC
[PR] Make plugin API cleanup functions noexcept [trafficserver]
bryancall opened a new pull request, #13590:
URL: https://github.com/apache/trafficserver/pull/13590
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
`UNCAUGHT_EXCEPT` is the single largest class of outstanding defects on the [Coverity Scan project](https://scan.coverity.com/projects/apache-traffic-server): 203 of 538 in the High Impact Outstanding view (build master-20260826).
Parsing the body of every flagged destructor, 83 findings are directly attributable to this change — 80 destructors whose only calls are functions this PR makes `noexcept`, plus 3 `TsSharedMutex` guard destructors covered by the `_call_fatal` fix. By area: 61 in plugins, 13 in `tscpp/api`, 5 in core, 2 in examples, 2 in test code. Representative CIDs: 1528663, 1528712, 1660590, 1660588, 1533104, 1528778.
The other 120 need separate root causes (the `main()` entry points in the CLI tools, and destructors whose throw path is a member subobject rather than an API call); those are follow-up changes, not this one.
### 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 — but it is a real change and worth a release note.
- **Source compatibility:** since C++17 `noexcept` is part of the function type, so any translation unit that includes `ts.h` *and* separately declares one of these functions without `noexcept` becomes ill-formed. 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make plugin API cleanup functions noexcept [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #13590:
URL: https://github.com/apache/trafficserver/pull/13590#issuecomment-5432139202
[approve ci autest 2]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make plugin API cleanup functions noexcept [trafficserver]
Posted by "cmcfarlen (via GitHub) via github" <gi...@trafficserver.apache.org>.
cmcfarlen commented on PR #13590:
URL: https://github.com/apache/trafficserver/pull/13590#issuecomment-6532923838
Cherry-picked to the 10.2.x branch as dc65088cd7 for the 10.2.2 release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make plugin API cleanup functions noexcept [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall merged PR #13590:
URL: https://github.com/apache/trafficserver/pull/13590
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make plugin API cleanup functions noexcept [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #13590:
URL: https://github.com/apache/trafficserver/pull/13590#issuecomment-5517972649
[approve ci autest 2]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org