fix: prevent data loss in eventbus by applying backpressure#2
Open
AdityaJadhav24 wants to merge 1 commit into
Open
fix: prevent data loss in eventbus by applying backpressure#2AdityaJadhav24 wants to merge 1 commit into
AdityaJadhav24 wants to merge 1 commit into
Conversation
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
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.
Description
I identified a logic flaw in the eventbus while auditing the codebase for security tools. The
dispatchfunction inbus.gocurrently uses a non-blockingselectstatement with adefaultcase.The Problem
In many applications, dropping events prevents system crashes, but for a security tool like God's Eye, it causes silent data loss. If the scanner discovers assets faster than a subscriber can process them, the tool increments the
droppedcounter and throws the data away. This means a user could receive a "complete" report that is actually missing critical findings.The Fix
I removed the
defaultblock indispatchto ensure the event bus applies backpressure. Instead of dropping data, the publisher will now pause and wait for the buffer to clear. This change guarantees 100% data integrity for every scan.Testing Updates
TestPublish_NonBlocking_DropsWhenBufferFull. This test is no longer valid because the architecture is no longer designed to drop events.TestPublish_Blocking_AppliesBackpressure. This new test confirms the publisher correctly blocks when the buffer is full and delivers all events once space is available.Important Tradeoff
By making this change, I am choosing accuracy over raw throughput. If a subscriber is slow, the entire scanning pipeline will slow down to match it. I believe this is the right choice for an auditing tool where missing a single vulnerability is a worse outcome than a longer scan time.