Add modsecurity_log_thread_pool to run phase 5 and audit logging in a thread pool

[tomsommer]

what

• New opt-in directive modsecurity_log_thread_pool <name> | off (http, server, location; default off). When set, the LOG phase handler posts msc_process_logging() — phase 5 rule evaluation plus the audit log write — to the named nginx thread_pool instead of running it on the worker's event loop. The transaction is detached from the request pool, processed in the thread, and released from the task's completion handler on the main thread. If the task cannot be posted (queue full, allocation failure) the connector falls back to the existing synchronous path and logs a warning.
• The libmodsecurity log callback now receives a small holder struct instead of r->connection->log, so its destination can be redirected to the thread pool's log once the request is gone. Rule messages emitted during phases 1–4 are unchanged; messages emitted from phase 5 in a thread go to the main error_log without the client/request suffix.
• Compiled only with NGX_THREADS and NGX_PCRE2; on other builds the directive is refused at configuration time with an explanatory message. The early logging path used by interventions stays synchronous.
• README section (marked experimental, with the operational caveats: file error_log targets only, in-flight tasks at worker shutdown, SecDebugLog written from the thread, libmodsecurity must be a release that supports concurrent transactions as used by threaded Apache MPMs).
• New test tests/modsecurity-log-thread-pool.t (11 assertions; skips without --with-threads or with --without-pcre2): blocking still works, phase 5 rules run and the audit log is written from the thread, a location with the directive off still logs synchronously, and the error log proves the thread path was taken (no fallback warning, phase 5 message without request suffix).

why

• msc_process_logging() writes the audit log synchronously inside the worker. With SecAuditEngine On, or RelevantOnly under attack traffic, that is per-request file I/O on the event loop, blocking every other connection of that worker (Synchronous process of the request destroys performance #227).
• Offloading only phase 5 keeps the request path unchanged: nothing about blocking decisions moves off the worker, and the thread never touches r, its pool, or its connection. Object lifetimes were walked for every path (sync, offloaded, offload failure, early intervention logging, internal redirect between locations with different settings, request termination, worker shutdown, reload) and the branch runs clean under valgrind with tasks in flight at quit.
• PCRE1 builds are excluded because the connector swaps the process-global pcre_malloc/pcre_free around every libmodsecurity call (see fix: document PCRE1 global state thread-safety hazard and add build w… #380); PCRE2 builds compile those helpers to no-ops.

references

• Addresses Synchronous process of the request destroys performance #227 (synchronous processing) for the logging half.
• Narrower alternative to Put the ModSecurity interception logic onto separated threads #289, which moves rule evaluation itself onto threads and is marked incomplete by its author; the two are independent.
• Related: fix: document PCRE1 global state thread-safety hazard and add build w… #380 (PCRE1 thread-safety hazard), fix: correctness gaps in response header/intervention handling #385 (owns the unused sizeof(ctx) cleanup allocation left untouched here), fix: fail closed on swallowed WAF return values #384 (owns the create_ctx error returns left untouched here).

---

Origin: this change comes from a performance review of the connector done with Claude Fable 5.1 (Anthropic). The patch and its test were verified by building the module against nginx master with libmodsecurity 3.0.14 (PCRE2, --with-threads, with and without --with-debug), without threads, and, with upstream CI's flags (--without-pcre2 --with-http_v2_module --with-http_auth_request_module), against libmodsecurity 3.0.9 (PCRE1), then running the full tests/modsecurity*.t suite in each build (16 files; 262 tests where the new test runs, 249 where it skips; all passing) and the new test under valgrind.

[coderabbitai]

Warning

Review limit reached

Next included review available in 14 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: af710b02-67a6-4f4b-80bf-b73945ce18b6

📥 Commits

Reviewing files that changed from the base of the PR and between 9eb44fd and 22b3d75.

📒 Files selected for processing (5)

• README.md
• src/ngx_http_modsecurity_common.h
• src/ngx_http_modsecurity_log.c
• src/ngx_http_modsecurity_module.c
• tests/modsecurity-log-thread-pool.t

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud