tls: prevent engine hang from spinning TLS read/write retry loop

[shaq918]

Summary

Fixes #9927 — Fluent Bit silently hangs (stops forwarding data) while appearing healthy.

When a TLS forward input connection dies (peer disconnects ungracefully), flb_tls_net_read_async() enters a tight spin loop that monopolizes the main engine thread and starves all output processing.

Root Cause

In flb_tls_net_read_async() (src/tls/flb_tls.c):

retry_read:
    ret = tls->api->net_read(session, buf, len);  // SSL_read() -> read(fd) -> EAGAIN
    if (ret == FLB_TLS_WANT_READ) {
        io_tls_event_switch(session, MK_EVENT_READ);  // no-op: already registered
        flb_coro_yield(co, FLB_FALSE);                // yields, immediately resumed
        goto retry_read;                               // infinite loop
    }

1. A dead TLS connection's socket stays TCP ESTABLISHED with rx_queue=0
2. SSL_read() -> read(fd) -> EAGAIN -> OpenSSL returns SSL_ERROR_WANT_READ
3. io_tls_event_switch() is a no-op because the event is already registered for MK_EVENT_READ
4. flb_coro_yield() yields, but epoll immediately resumes the coroutine (level-triggered, event never consumed)
5. goto retry_read -> step 2 -> tight spin, ~8,500 iterations/second

This monopolizes the flb-pipeline thread. All other event processing is starved: output flushes never execute, scheduler timers don't fire, chunks stay permanently busy.

Fix

Add FLB_TLS_WANT_READ_MAX_RETRIES (800) to cap consecutive WANT_READ/WANT_WRITE iterations with no progress in both flb_tls_net_read_async() and flb_tls_net_write_async(). When exceeded, return -1 to close the stale connection and unblock the engine thread.

Evidence from Production (Fluent Bit v3.2.2, Ubuntu 22.04, OpenSSL 3.0.2)

Strace on stuck flb-pipeline thread (the main engine thread):

$ strace -p <pipeline-tid> -c (2 seconds)
% time     seconds  usecs/call     calls    errors syscall
100.00    0.088144           5     17029     17029 read

17,029 failed read() calls in 2 seconds — 100% EAGAIN, zero useful work.

The spinning fd is a dead TCP socket:

$ readlink /proc/<pid>/fd/89
socket:[243422702]

$ ss -tnei src :24224 dst <peer> dport = 36120
ESTAB 0 0  local:24224  peer:36120
    lastrcv:314289920   (~3.6 days since last data)

$ grep <inode> /proc/net/tcp
... 01 00000000:00000000 ...    (ESTABLISHED, rx_queue=0, tx_queue=0)

Output counter frozen:

fluentbit_output_proc_records_total{name="stackdriver.0"} 9123353  (does not increment)
fluentbit_output_errors_total{name="stackdriver.0"} 0              (output never gets to run)

Storage API — chunks stuck busy:

{"tail.0": {"chunks": {"total":4, "busy":3}}}

Live Validation

Killing the dead TCP connections (simulating what this fix does) immediately recovered the pipeline without a restart:

Step	output_proc_records	Engine thread
Before (stuck 3 days)	9,123,353 (frozen)	100% read(), 8,500 EAGAIN/s
After killing dead connections	9,264,379 (+141,026)	Healthy: epoll_wait, read, write, newfstatat

Before (stuck):

$ strace -p <pipeline-tid> -c (2 seconds)
% time     seconds  usecs/call     calls    errors syscall
100.00    0.088144           5     17029     17029 read

After killing dead connections:

$ strace -p <pipeline-tid> -c (2 seconds)
% time     seconds  usecs/call     calls    errors syscall
 44.80    0.006476           5      1100         2 newfstatat
 14.51    0.002097           6       318         6 read
 10.91    0.001577           7       223           epoll_wait
  8.27    0.001195          17        70           fadvise64
  5.17    0.000747           6       108           write
  ...

Same pattern confirmed on a second independent node with a different dead connection (47 days idle, different peer IP/port). Multiple dead connections can stack — we had to close 4-5 before the engine fully recovered.

Files Changed

• include/fluent-bit/tls/flb_tls.h — add FLB_TLS_WANT_READ_MAX_RETRIES constant (800)
• src/tls/flb_tls.c — add retry counter to flb_tls_net_read_async() and flb_tls_net_write_async()

Summary by CodeRabbit

• Bug Fixes
  • Added safeguards to prevent stalled TLS read/write operations from looping indefinitely.
  • Connections that repeatedly return “want read” or “want write” now time out after too many retries, improving stability and reducing deadlocks.
  • Retry handling now resets appropriately when progress is made, helping active connections continue normally.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new FLB_TLS_WANT_READ_MAX_RETRIES constant (800) and applies it in both flb_tls_net_read_async and flb_tls_net_write_async to detect connections stalled in repeated WANT_READ/WANT_WRITE states, marking them as timed out (ETIMEDOUT) and aborting the coroutine instead of looping indefinitely.

Changes

TLS Async Retry Timeout Handling

Layer / File(s)	Summary
Retry limit constant include/fluent-bit/tls/flb_tls.h	Defines FLB_TLS_WANT_READ_MAX_RETRIES (800) with a comment documenting its use to detect stale TLS connections.
Async read timeout detection src/tls/flb_tls.c	Adds <errno.h> include and a want_read_retries counter in flb_tls_net_read_async that resets on WANT_WRITE and triggers a timeout closeout (warning log, cleared coroutine, net_error = ETIMEDOUT, restored event registration, return -1) when the retry limit is exceeded.
Async write timeout detection src/tls/flb_tls.c	Adds a want_retries counter in flb_tls_net_write_async that increments on repeated WANT_WRITE or WANT_READ and applies the same timeout closeout, updating *out_len to the total bytes written so far.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Coroutine
  participant TLSAsyncIO as flb_tls_net_read_async/write_async
  participant EventLoop

  Coroutine->>TLSAsyncIO: perform TLS read/write
  TLSAsyncIO->>TLSAsyncIO: attempt operation
  alt WANT_READ/WANT_WRITE returned
    TLSAsyncIO->>TLSAsyncIO: increment retry counter
    alt retries <= FLB_TLS_WANT_READ_MAX_RETRIES
      TLSAsyncIO->>EventLoop: yield and wait for event
      EventLoop-->>Coroutine: resume coroutine
    else retries exceeded
      TLSAsyncIO->>TLSAsyncIO: set net_error = ETIMEDOUT, clear coroutine
      TLSAsyncIO->>EventLoop: restore backed-up event registration
      TLSAsyncIO-->>Coroutine: return -1
    end
  else operation completes
    TLSAsyncIO-->>Coroutine: return bytes read/written
  end

Loading

Suggested labels: backport to v4.2.x

Suggested reviewers: cosmo0920, fujimotos

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly states the main fix: preventing TLS read/write retry loops from hanging the engine.
Linked Issues check	✅ Passed	The changes cap repeated TLS WANT_READ/WANT_WRITE retries, which addresses the hang described in #9927.
Out of Scope Changes check	✅ Passed	The patch stays focused on TLS retry-loop handling and adds no obvious unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 54019a9639

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Cap WANT_WRITE retries during TLS reads

When SSL_read() returns FLB_TLS_WANT_WRITE, this branch resets the only retry counter and then retries without any limit. For a stalled TLS read that needs the socket write side, or alternates WANT_READ/WANT_WRITE without returning bytes, the coroutine can still bounce through flb_coro_yield() indefinitely and monopolize the engine thread; count WANT_WRITE as a no-progress retry too, or reset only after a successful read.

Useful? React with 👍 / 👎.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/tls/flb_tls.c (1)

540-631: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Reset want_retries after forward progress.
tls->api->net_write() can return a positive partial count, and this loop keeps sending the remaining bytes. Without resetting want_retries on success, a long but healthy transfer can accumulate more than 800 WANT_READ/WANT_WRITE events over the same send and get closed as stale.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/tls/flb_tls.c` around lines 540 - 631, The retry counter in
flb_tls_net_write can incorrectly carry across successful partial writes,
causing healthy long sends to be treated as stale; update the retry handling in
the flb_tls_net_write loop so want_retries is reset after any positive progress
from tls->api->net_write, while keeping the existing retry limits and event
switching behavior intact.

🧹 Nitpick comments (1)

include/fluent-bit/tls/flb_tls.h (1)

38-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Constant name is read-specific but reused on the write path.

FLB_TLS_WANT_READ_MAX_RETRIES is also used to bound WANT_WRITE/write retries in flb_tls_net_write_async, which makes the name misleading at those call sites. Consider a neutral name such as FLB_TLS_WANT_IO_MAX_RETRIES.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@include/fluent-bit/tls/flb_tls.h` around lines 38 - 43, The retry limit
constant name is too read-specific for a value that is also used by write retry
logic, so rename FLB_TLS_WANT_READ_MAX_RETRIES in flb_tls.h to a neutral
I/O-oriented symbol and update all call sites such as flb_tls_net_write_async
and any related TLS retry checks to use the new name consistently.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/tls/flb_tls.c`:
- Around line 540-631: The retry counter in flb_tls_net_write can incorrectly
carry across successful partial writes, causing healthy long sends to be treated
as stale; update the retry handling in the flb_tls_net_write loop so
want_retries is reset after any positive progress from tls->api->net_write,
while keeping the existing retry limits and event switching behavior intact.

---

Nitpick comments:
In `@include/fluent-bit/tls/flb_tls.h`:
- Around line 38-43: The retry limit constant name is too read-specific for a
value that is also used by write retry logic, so rename
FLB_TLS_WANT_READ_MAX_RETRIES in flb_tls.h to a neutral I/O-oriented symbol and
update all call sites such as flb_tls_net_write_async and any related TLS retry
checks to use the new name consistently.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 36038539-77bc-47ed-a991-7fc5abd58c12

📥 Commits

Reviewing files that changed from the base of the PR and between 46e15a1 and 54019a9.

📒 Files selected for processing (2)

• include/fluent-bit/tls/flb_tls.h
• src/tls/flb_tls.c

[shaq918]

Note: PR #11547 addresses the same root cause (infinite WANT_READ/WANT_WRITE retry loops) but only fixes the synchronous paths (flb_tls_net_read and flb_tls_net_write). This PR fixes the asynchronous paths (flb_tls_net_read_async and flb_tls_net_write_async), which is where the production hang occurs — the forward input plugin uses async I/O, and the spin happens through the coroutine yield/resume loop, monopolizing the main engine thread.

Both PRs are complementary and needed for full coverage.