janiussyafiq opened a new pull request, #13232:
URL: https://github.com/apache/apisix/pull/13232

   ### Description
   
   When `apisix.tracing` is enabled, the core tracer instruments every request 
phase — including `ssl_client_hello_phase` — by allocating a tracing table from 
`lua-tablepool` and storing it in `ngx.ctx.tracing`. On HTTPS keepalive 
connections, OpenResty reuses the same `ngx.ctx` object across multiple HTTP 
requests on the same TLS session.
   
   The bug occurs in the following sequence:
   
   1. `ssl_client_hello_phase` calls `tracer.start()`, which allocates 
`ctx.tracing` via tablepool and initialises `tracing.spans`.
   2. The first HTTP request completes and `tracer.release()` is called in the 
log phase, returning the tracing table to the pool. `lua-tablepool` zeroes all 
fields on release — `tracing.spans` becomes `nil` — but `ctx.tracing` is never 
cleared, leaving a stale non-nil pointer to the zeroed table.
   3. On the second HTTP request (same keepalive connection), `tracer.start()` 
finds `ctx.tracing` is non-nil (a zeroed table is still truthy in Lua) and 
skips re-initialisation.
   4. `span.new()` then crashes at `table.insert(tracing.spans, self)` because 
`spans` is `nil`.
   
   This fix addresses the root cause at two layers:
   
   - **`tracer.release()`**: `ctx.tracing = nil` is now cleared at the very 
beginning of the function, before any tablepool operations, so a stale pointer 
is never left in `ngx.ctx` for the next request to inherit. A `if spans then` 
guard is also added to make release safe when called on an 
already-partially-cleared table.
   - **`tracer.start()`**: The initialisation guard is extended from `if not 
tracing then` to `if not tracing or not tracing.spans then`, so tracing is 
correctly re-initialised even if a stale state is somehow encountered (e.g. 
diverged HTTP/2 contexts).
   
   #### Which issue(s) this PR fixes:
   
   Fixes #13200 
   
   ### Checklist
   
   - [x] I have explained the need for this PR and the problem it solves
   - [x] I have explained the changes or the new features added to this PR
   - [x] I have added tests corresponding to this change (`t/node/tracer.t`)
   - [ ] I have updated the documentation to reflect this change
   - [x] I have verified that this change is backward compatible


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to