elastic / elastic/beats

[libbeat-pipeline-lifecycle] diskqueue writer panic during shutdown when segment open retry aborts

Open
#49,608 1 comment 0 reactions 0 assignees View on GitHub
needs_team
Dominant language
Go
Stars
12.7k
Forks
5k
Avg merge
2d 15m
Merged PRs (30d)
385

Description

## Findings

### 1) Nil-pointer panic in diskqueue writer shutdown/error path (high)

**Location:** `libbeat/publisher/queue/diskqueue/writer_loop.go` lines `181-186`, `242`

**Evidence:**

- When opening a new segment writer fails, `processRequest` aborts the frame loop:

`libbeat/publisher/queue/diskqueue/writer_loop.go:181-186`
```go
file, err := wl.currentSegment.getWriterWithRetry(...)
if err != nil {
// This can only happen if the queue is being closed; abort.
break
}
```

- After the loop, code unconditionally calls `Sync()` on `wl.outputFile`:

`libbeat/publisher/queue/diskqueue/writer_loop.go:242`
```go
_ = wl.outputFile.Sync()
```

- `wl.outputFile` is nullable (`*segmentWriter`), and is set only after successful open (`line 190`). If open fails and the loop breaks before assignment, `wl.outputFile` can be `nil`.

**What is wrong:**
During shutdown while writer-open retries are being aborted, `processRequest` can reach `wl.outputFile.Sync()` with `wl.outputFile == nil`, causing a panic in the segment writer method call path.

**Why it matters:**
This is a high-severity lifecycle failure: a normal operational sequence (queue shutdown while disk/open failures are occurring) can crash Beat during graceful shutdown, risking incomplete drain/ACK handling and interrupted cleanup.

**Suggested fix:**
- Guard the final sync with nil-check:
- `if wl.outputFile != nil { _ = wl.outputFile.Sync() }`
- Keep response accounting/ACK behavior consistent for partial writes after abort.
- Add a regression test that forces writer-open retry failure then triggers shutdown, asserting no panic and successful close completion.

**Reproduction direction (deterministic):**
1. Configure diskqueue path/permissions so segment open fails (or inject `getWriterWithRetry` failure via test hook).
2. Start queue with pending write frames requiring a new segment.
3. Trigger shutdown so `writerLoop.requestChan` abort path is taken.
4. Assert shutdown does not panic and `Done()` closes.

## Suggested Actions

- [ ] Add nil-guard around `wl.outputFile.Sync()` in `writer_loop.go`.
- [ ] Add regression test for shutdown + open-retry-abort path.
- [ ] Run `go test -race ./libbeat/publisher/queue/diskqueue/...` and `go test -race ./libbeat/publisher/...` to validate lifecycle behavior.

## Audited areas found clean in this pass

- Pipeline signal broadcast patterns in `libbeat/publisher/pipeline/` (shutdown signaling uses close/broadcast patterns appropriately).
- Backoff wait cancellation mechanism uses `done` channel in backoff waits (no concrete high-severity broadcast bug confirmed in this pass).
- No duplicate open issue found for this finding (checked previous findings and current open issues with this workflow prefix).

---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Sweeper: Libbeat Pipeline Shutdown and Queue Lifecycle](https://github.com/elastic/beats/actions/runs/23450916099)

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
> - [x] expires on Mar 30, 2026, 5:42 PM UTC

Contributor guide

Open the contributing guide

Research direction

Start in libbeat/publisher/queue/diskqueue/writer_loop.go, especially processRequest and the shutdown path around lines 181-186 and 242. Add a regression test that forces writer-open retry failure during shutdown, then run go test -race ./libbeat/publisher/queue/diskqueue/... and go test -race ./libbeat/publisher/.... Done means shutdown completes without a panic and Done() closes.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
65/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.