bryancall commented on issue #13496:
URL:
https://github.com/apache/trafficserver/issues/13496#issuecomment-5429177939
Thanks for the report, and for digging into it — your read is correct, and
the analysis saved me a lot of time.
To answer your question directly: **yes, `s->current.server` can be null at
that point.** There's a path through `find_server_and_update_current_info()`
that reaches `build_request()` without it being set.
The function has two exits that both mean "no upstream available", and they
don't behave identically:
- The `PARENT_FAIL` case sets `s->current.request_to = HOST_NONE` **and**
returns `HOST_NONE`.
- The `PARENT_DIRECT` case, when
`proxy.config.http.no_dns_just_forward_to_parent` is enabled, sets
`s->parent_result.result = PARENT_FAIL` and **returns** `HOST_NONE` — but never
assigns `s->current.request_to`.
`HandleCacheOpenReadHit` discards the return value and tests the member
instead, requiring both conditions:
```cpp
else if (s->current.request_to == HOST_NONE && s->parent_result.result ==
PARENT_FAIL) {
```
The second exit only satisfies half of that, so the guard is skipped,
`server_up` stays `true`, and `build_request()` is called at line 2961 —
outside the `s->current.server != nullptr` check at 2931 that you spotted.
`handle_request_keep_alive_headers()` then dereferences it at 6889.
Two details that make me fairly confident this is your crash. First, the
other `build_request()` call sites pass `s->current.server->http_version` as an
argument, so a null there would fault in the calling frame — your trace faults
*inside* `build_request`, which only fits the one call site that uses a local
`http_version`. Second, the sibling callers `LookupSkipOpenServer` and the
cache-miss path both check `parent_result.result == PARENT_FAIL` on its own and
call `handle_parent_died()`; `HandleCacheOpenReadHit` is the only one that adds
the `request_to` condition.
This is still present on current master, and it isn't a dangling pointer
from the reload — `parent_params` is reference counted per transaction, so the
config object stays alive. My working theory on the reload timing is that the
newly loaded table returns `DIRECT` for a host that previously matched a parent
rule, which is what steers you into that second exit.
Could you confirm two things?
1. Do you have `proxy.config.http.no_dns_just_forward_to_parent` set to `1`?
The path requires it, and it's `0` by default.
2. Just before the crash, does your `diags.log` or `error.log` contain `no
available parents and the config
proxy.config.http.no_dns_just_forward_to_parent, prevents origin lookups.`?
That warning is emitted on exactly this branch, so it would confirm it.
If both hold, that settles it and I'll put up a fix — most likely making
that exit set `request_to` like its sibling does, so the existing guard catches
it. Labeling as a bug so we can track it.
--
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]
