redpanda-data / redpanda-data/benthos

Goroutine leak in service.Stream.Run

Open
#325 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
571
Forks
121
Avg merge
2d 1h
Merged PRs (30d)
18

Description

When use service.Stream.Run
https://github.com/redpanda-data/benthos/blob/main/public/service/stream.go#L72

func (s *Stream) Run(ctx context.Context) (err error) {
	s.strmMut.Lock()
	if s.strm != nil {
		err = errors.New("stream has already been run")
	} else {
		s.strm, err = stream.New(s.conf, s.mgr,
			stream.OptOnClose(func() {
				s.shutSig.TriggerHasStopped()
			}))
	}
//...
}

the stream.New will be called
https://github.com/redpanda-data/benthos/blob/main/internal/stream/type.go#L53

func New(conf Config, mgr bundle.NewManagement, opts ...func(*Type)) (*Type, error) {
	t := &Type{
		conf:    conf,
		manager: mgr,
		onClose: func() {},
		closed:  0,
	}
	for _, opt := range opts {
		opt(t)
	}
	if err := t.start(); err != nil {
		// bug in here!
		// when we get an error, we need do some clean up
		return nil, err
	}
// ...
}

Because in stream.Type.start we construct all layer.
If we construct the input layer success, and the input loop will be run in background.

Then, we construct the output layer fail, the func just returned, and the input loop leaked.

https://github.com/redpanda-data/benthos/blob/main/internal/stream/type.go#L141

func (t *Type) start() (err error) {
	// Constructors
	iMgr := t.manager.IntoPath("input")
	// bug in here
	// the input loop start when the input component constructed
	// https://github.com/redpanda-data/benthos/blob/main/internal/component/input/async_reader.go#L68
	if t.inputLayer, err = iMgr.NewInput(t.conf.Input); err != nil {
		return
	}
	if t.conf.Buffer.Type != "none" {
		bMgr := t.manager.IntoPath("buffer")
		if t.bufferLayer, err = bMgr.NewBuffer(t.conf.Buffer); err != nil {
			return
		}
	}
	if tLen := len(t.conf.Pipeline.Processors); tLen > 0 {
		pMgr := t.manager.IntoPath("pipeline")
		if t.pipelineLayer, err = pipeline.New(t.conf.Pipeline, pMgr); err != nil {
			return
		}
	}
	oMgr := t.manager.IntoPath("output")
	if t.outputLayer, err = oMgr.NewOutput(t.conf.Output); err != nil {
		return
	}
// ...
}

And also, other layers loop start when the Consume interface been called.
We also need to handle their clean up work when some error happened.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with public/service/stream.go, internal/stream/type.go, and internal/component/input/async_reader.go, then trace how each layer starts and is consumed. Reproduce or inspect failures during input, buffer, pipeline, or output construction; done means partially constructed layers and their background loops are cleaned up when a later constructor fails.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
stream-processing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.