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

Jump to bottom

Fix goroutine leak in local.file #6347

Closed
wants to merge 1 commit into from
Closed

Fix goroutine leak in local.file #6347

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 13 additions & 3 deletions component/local/file/file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ func New(o component.Options, args Arguments) (*Component, error) {
}
// Perform an update which will immediately set our exports to the initial
// contents of the file.
if err = c.Update(args); err != nil {
// UpdateArgs instead of Update to prevent starting goroutines if Run() is not called.
if err = c.UpdateArgs(args); err != nil {
return nil, err
}
return c, nil
Expand Down Expand Up @@ -184,8 +185,7 @@ func (c *Component) readFile() error {
return nil
}

// Update implements component.Component.
func (c *Component) Update(args component.Arguments) error {
func (c *Component) UpdateArgs(args component.Arguments) error {
newArgs := args.(Arguments)

if newArgs.PollFrequency <= 0 {
Expand All @@ -200,6 +200,16 @@ func (c *Component) Update(args component.Arguments) error {
if err := c.readFile(); err != nil {
return fmt.Errorf("failed to read file: %w", err)
}
return nil
}

// Update implements component.Component.
func (c *Component) Update(args component.Arguments) error {

err := c.UpdateArgs(args)
if err != nil {
return err
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think Run() could be called without calling Update(). In that case, is the fact that we didn't call c.configureDetector() prior to Run() a problem?


// Each detector is dedicated to a single file path. We'll naively shut down
// the existing detector (if any) before setting up a new one to make sure
Expand Down
Loading