Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(tmplexec): handle race condition on tmplexec #5885

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

fix(tmplexec): handle race condition on tmplexec #5885

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
82c3f9a
fix(tmplexec): handle rare case where lastMatcherEvent address change…
babariviere Dec 5, 2024
cbe27fb
Merge remote-tracking branch 'origin' into pr/5885
ehsandeep Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions pkg/tmplexec/exec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,12 @@ func (e *TemplateExecuter) Execute(ctx *scan.ScanContext) (bool, error) {
ctx.LogError(errx)

if lastMatcherEvent != nil {
lastMatcherEvent.Lock()
lastMatcherEvent.InternalEvent["error"] = getErrorCause(ctx.GenerateErrorMessage())
lastMatcherEvent.Unlock()
writeFailureCallback(lastMatcherEvent, e.options.Options.MatcherStatus)
// copy lastMatcherEvent pointer to avoid race condition where lastMatcherEvent is changed after the lock.
event := lastMatcherEvent
event.Lock()
event.InternalEvent["error"] = getErrorCause(ctx.GenerateErrorMessage())
event.Unlock()
writeFailureCallback(event, e.options.Options.MatcherStatus)
Comment on lines +217 to +222
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Refactor the workaround for handling unmatched events

The current implementation includes a workaround to handle cases where the callback is not called and matcher-status is true, as indicated by the TODO comment. This approach may lead to maintenance challenges and unexpected behaviors.

Consider refactoring this section to wait for the ScanContext to complete before proceeding. This would ensure proper synchronization and more robust handling of unmatched events.

Would you like assistance in proposing a refactored implementation for this section?

}

//TODO: this is a hacky way to handle the case where the callback is not called and matcher-status is true.
Expand Down
Loading