fix: prevent data loss on interrupt by implementing graceful shutdown#2393
Open
assakafpix wants to merge 1 commit intoprojectdiscovery:devfrom
Open
fix: prevent data loss on interrupt by implementing graceful shutdown#2393assakafpix wants to merge 1 commit intoprojectdiscovery:devfrom
assakafpix wants to merge 1 commit intoprojectdiscovery:devfrom
Conversation
WalkthroughImplements a two-stage Ctrl+C shutdown: first interrupt signals the Runner to stop dispatching new work and drain in-flight requests, deferring cleanup; a second Ctrl+C forces exit. Adds resume-persistence after an interrupted run when resume saving is enabled, and exposes Runner.Interrupt() and Runner.IsInterrupted(). Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as CLI Handler
participant Runner as Runner
participant Req as In-flight Requests
participant FS as File System
User->>CLI: Ctrl+C (first)
CLI->>Runner: Interrupt()
Note over Runner: Close interruptCh / mark interrupted
Runner->>Runner: Stop dispatching new items
Runner->>Req: Allow in-flight requests to complete
Req-->>Runner: Responses complete
Runner-->>CLI: RunEnumeration returns
CLI->>Runner: IsInterrupted()?
alt interrupted and resume enabled
CLI->>FS: SaveResumeConfig()
FS-->>CLI: Resume file created / error
end
User->>CLI: Ctrl+C (second)
CLI->>CLI: Force exit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@runner/runner.go`:
- Around line 1475-1480: The consumer breaks out when r.IsInterrupted() but the
producer (streamInput) may still be sending to streamChan (unbuffered), causing
a deadlock; update the producer(s) that do "out <- item" (e.g., streamInput
goroutine) to be interrupt-aware by selecting on a cancellation signal (use
r.IsInterrupted() via a done channel or context) before sending, and stop/return
when interrupted, or close the channel from a single owner before exit so the
consumer can range-finish; ensure all places that write "out <- item" use this
select pattern to avoid blocking sends after interruption.
🧹 Nitpick comments (2)
runner/runner.go (1)
107-124: Make Interrupt safe under concurrent calls.If
Interrupt()is ever called from multiple goroutines, the currentclosepattern can double-close the channel and panic. Async.Once(or atomic CAS) makes this robust.🔧 Suggested fix
type Runner struct { ... - interruptCh chan struct{} + interruptCh chan struct{} + interruptOnce sync.Once } func (r *Runner) Interrupt() { - select { - case <-r.interruptCh: - default: - close(r.interruptCh) - } + r.interruptOnce.Do(func() { close(r.interruptCh) }) }runner/runner_test.go (1)
18-84: Add runner cleanup and use the public interrupt API.Closing runners avoids leaking hybrid-map/ratelimiter resources across tests, and
IsInterrupted()keeps the test aligned with the public API rather thaninterruptChinternals.♻️ Suggested fix
rFull, err := New(&Options{}) require.Nil(t, err, "could not create httpx runner") +t.Cleanup(rFull.Close) ... rInt, err := New(&Options{}) require.Nil(t, err, "could not create httpx runner") +t.Cleanup(rInt.Close) ... - select { - case <-rInt.interruptCh: - continue - default: - } + if rInt.IsInterrupted() { + continue + } ... rRes, err := New(&Options{}) require.Nil(t, err, "could not create httpx runner") +t.Cleanup(rRes.Close)
assakafpix
force-pushed
the
fix/resume-lost-targets
branch
from
32a8306 to
fa95023
Compare
|
doesn't seem to be working from my tests: ayux@pop-os:~/httpx$ echo -e "example.com\ntest.com\nfacebook.com\nhackerone.com\ngoogle.com\n" | go run cmd/httpx/httpx.go
__ __ __ _ __
/ /_ / /_/ /_____ | |/ /
/ __ \/ __/ __/ __ \| /
/ / / / /_/ /_/ /_/ / |
/_/ /_/\__/\__/ .___/_/|_|
/_/
projectdiscovery.io
[INF] Current httpx version v1.8.1 (latest)
[WRN] UI Dashboard is disabled, Use -dashboard option to enable
https://example.com
https://google.com
https://facebook.com
^C[INF] CTRL+C pressed: Exiting
https://hackerone.com
[INF] Creating resume file: resume.cfg
[ble: exit 1]
ayux@pop-os:~/httpx$ cat resume.cfg
resume_from=test.com
index=5
ayux@pop-os:~/httpx$ echo -e "example.com\ntest.com\nfacebook.com\nhackerone.com\ngoogle.com\n" | go run cmd/httpx/httpx.go -r resume.cfg -v
__ __ __ _ __
/ /_ / /_/ /_____ | |/ /
/ __ \/ __/ __/ __ \| /
/ / / / /_/ /_/ /_/ / |
/_/ /_/\__/\__/ .___/_/|_|
/_/
projectdiscovery.io
[INF] Current httpx version v1.8.1 (latest)
[DBG] Using resolvers: resume_from=test.com,index=5
[WRN] UI Dashboard is disabled, Use -dashboard option to enable
[DBG] Failed 'http://facebook.com': Get "http://facebook.com": cause="no address found for host"
[DBG] Failed 'http://test.com': Get "http://test.com": cause="no address found for host"
[DBG] Failed 'http://hackerone.com': Get "http://hackerone.com": cause="no address found for host"
[DBG] Failed 'http://google.com': Get "http://google.com": cause="no address found for host"
[DBG] Failed 'http://example.com': Get "http://example.com": cause="no address found for host"
ayux@pop-os:~/httpx$ git branch
* fix/resume-lost-targets |
Author
|
Hi @ayuxsec ! |
|
oh mb! edit: though index of test.com was also wrong in resume.cfg you might wanna check on that |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
/claim #2345
When httpx is interrupted with Ctrl+C,
resume.cfgsaves an index based on how many targets were dispatched to the thread pool, not how many actually completed and producedoutput. With the default 50 threads, the scanner races through all inputs before any HTTP response returns, so
resume.cfgalways points near the end of the list even if almostnothing was written to output. On resume with
-resume(not-r, which is the flag for custom resolvers), all those dispatched-but-incomplete targets are permanently skipped.The root cause is in
runner/runner.go:1406-1407currentIndexis incremented the moment a target is read from the input hash map, before the HTTP request even starts. Then theSIGINT handler in
cmd/httpx/httpx.gosaves this inflated index and callsos.Exit(1)immediately, without waiting for in-flight goroutines to finish.Proposed Changes
runner/runner.go: Add aninterruptChchannel toRunner. On Ctrl+C,processItemdetects the closed channel and stops dispatching new items (without incrementingcurrentIndex). The existingwg.Wait()inRunEnumerationnaturally drains all in-flight requests before returning.cmd/httpx/httpx.go: The SIGINT handler now callsInterrupt()instead ofSaveResumeConfig()+os.Exit(1). Resume saving moves to afterRunEnumeration()returns, whencurrentIndexaccurately reflects completed work. A second Ctrl+C force-exits.runner/runner_test.go: Test proving that interrupted output + resumed output = full scan with no gaps.Proof
Before (v1.8.1) — 108 inputs, 51 targets lost:
After — 108 inputs, 0 targets lost:
Checklist
dev)Summary by CodeRabbit
New Features
Tests