lostluck commented on a change in pull request #15815:
URL: https://github.com/apache/beam/pull/15815#discussion_r737589733
##########
File path: sdks/go/pkg/beam/core/runtime/harness/statemgr.go
##########
@@ -107,10 +105,6 @@ func (s *ScopedStateReader) Close() error {
s.mu.Lock()
s.closed = true
s.mgr = nil
- for _, r := range s.opened {
- r.Close() // force close all opened readers
Review comment:
So this PR isn't ensuring all readers are closed in user panic/error
cases like the flawed earlier approach does. A user panic or a dofn returning
an error will leave readers open, causing other leaks.
Something also needs to change in exec/pardo.go to close any open streams in
that case, using a defer to cover panic handling cases.
Essentially current change only works for happy path cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
