synserver accept assertion to log event details before aborting#12903
synserver accept assertion to log event details before aborting#12903bryancall merged 3 commits intoapache:masterfrom
Conversation
Replace bare TSAssert in synserver_vc_accept and synserver_vc_refuse with diagnostic ink_abort that logs the unexpected event number and, if the data looks like a negated errno, the strerror string. This gives CI output useful for debugging the accept() failure seen on Rocky (build apache#8376) where the synserver received an unexpected event after SDK_API_HttpParentProxySet_Success passed.
bryancall
changed the title
There was a problem hiding this comment.
Pull request overview
Improves diagnostics for unexpected events in the synserver_vc_accept / synserver_vc_refuse continuations inside InkAPITest, so future CI crashes provide actionable details (event id and possible errno) before aborting.
Changes:
- Replaced a bare
TSAssertwith anink_abort-based diagnostic path insynserver_vc_refuse. - Replaced a bare
TSAssertwith anink_abort-based diagnostic path insynserver_vc_accept. - Added best-effort decoding of
dataas a negated errno for improved logging.
| int err = -static_cast<int>(reinterpret_cast<intptr_t>(data)); | ||
| if (err > 0 && err < 256) { |
There was a problem hiding this comment.
data is treated as a negated errno via reinterpret_cast<intptr_t>(data) and then narrowed to int before negation. If data is actually a pointer (plausible for many events), the narrowing conversion can wrap/lose bits and accidentally produce a small positive value (e.g., <256), leading to a misleading strerror() message. Consider keeping the value as intptr_t, first checking it’s in a plausible negated-errno range (e.g., < 0 and >= -4095), and only then converting to an int errno; otherwise log the raw integer/pointer value.
| int err = -static_cast<int>(reinterpret_cast<intptr_t>(data)); | |
| if (err > 0 && err < 256) { | |
| intptr_t data_val = reinterpret_cast<intptr_t>(data); | |
| if (data_val < 0 && data_val >= -4095) { | |
| int err = static_cast<int>(-data_val); |
Check data in intptr_t space (negative and >= -4095) before treating it as a negated errno. Avoids truncating a pointer to int which could accidentally land in the errno range and print a misleading strerror.
|
[approve ci autest 1] |
1 similar comment
|
[approve ci autest 1] |
bneradt
left a comment
bneradt
left a comment
There was a problem hiding this comment.
lgtm. hopefully this helps us make progress on this.
|
Cherry-picked to 10.2.x |
* Replace bare TSAssert in synserver_vc_accept and synserver_vc_refuse with ink_abort that logs the unexpected event number and, if the data looks like a negated errno, the strerror string. Gives CI output useful for debugging the accept() failure seen on Rocky build #8376. * Validate data in intptr_t space (negative and >= -4095) before treating it as a negated errno to avoid truncating a pointer to int. (cherry picked from commit 33dee5d)
Problem
Rocky CI build #8376 crashed with a core dump in
synserver_vc_acceptafterSDK_API_HttpParentProxySet_Successpassed. The bareTSAssertlogged no information about which event was actually received, making diagnosis impossible.Changes
TSAssertwith diagnosticink_abort-- logs the unexpected event number and, if the data looks like a negated errno, thestrerrorstring in bothsynserver_vc_acceptandsynserver_vc_refuseintptr_tbefore narrowing to errno -- checks the range (-1 to -4095) in pointer-width space to avoid truncating a pointer that accidentally lands in the errno rangeTesting