Guard pre-init log wakeups - #13472
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request prevents a startup crash when plugins write to text logs during TSPluginInit, before logging preprocessing threads (and their EventNotify objects) are created. It does so by allowing buffers to be queued without signaling until the notification array exists, and adds an AuTest regression that forces an early rollover to validate the fix.
Changes:
- Guard
Log::preproc_notify[idx].signal()calls so early text-log buffer flush/rollover can’t dereference a nullpreproc_notify. - Extend the
test_log_interfacetest plugin to optionally write large log entries during plugin initialization. - Add a new gold test that configures a small log buffer, triggers initialization-time rollover, and verifies ATS stays running and the message reaches the plugin log.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/proxy/logging/LogObject.cc |
Adds null checks before signaling preproc_notify when enqueuing buffers for preprocessing, preventing pre-init crashes. |
tests/tools/plugins/test_log_interface.cc |
Adds a --write-during-init option to generate large initialization-time log writes to reproduce the pre-init rollover scenario. |
tests/gold_tests/logging/log_plugin_init.test.py |
New regression test to ensure initialization-time text logging doesn’t crash and buffered entries are flushed to the plugin log. |
Plugins can fill a text log buffer during TSPluginInit before logging preprocessing threads and their notification objects exist. This causes a null dereference that crashes traffic_server during startup. This problem is addressed in this patch by queuing those buffers without signaling until the logging workers exist. Their initial queue scan then flushes the pending data. A regression test forces a rollover during plugin initialization and verifies ATS stays up and preserves the log entry. With the production fix removed, the regression test caused traffic_server to crash with SIGSEGV at address zero while signaling the preprocessing notification. With the fix applied, the identical test passes.
35057c5 to
0b37ebc
Compare
|
I verified the regression test in an A/B run. With the test and test-plugin changes present but the production LogObject guards removed, traffic_server crashed during TSPluginInit with SIGSEGV at address 0x0. The backtrace passed through EventNotify::signal() and LogObject::_checkout_write(), and AuTest failed. Restoring the production fix made the identical test pass. |
Plugins can fill a text log buffer during TSPluginInit before logging
preprocessing threads and their notification objects exist. This causes
a null dereference that crashes traffic_server during startup.
This problem is addressed in this patch by queuing those buffers without
signaling until the logging workers exist. Their initial queue scan then
flushes the pending data. A regression test forces a rollover during
plugin initialization and verifies ATS stays up and preserves the log
entry.
With the production fix removed, the regression test caused
traffic_server to crash with SIGSEGV at address zero while signaling the
preprocessing notification. With the fix applied, the identical test
passes.