zeroshade commented on code in PR #1803:
URL: https://github.com/apache/iceberg-go/pull/1803#discussion_r3856614052
##########
table/scanner.go:
##########
@@ -1231,14 +1344,29 @@ func (scan *Scan) ReadTasks(ctx context.Context, tasks
[]FileScanTask) (*arrow.S
// closePlanIO releases the scoped resources associated with the current
// remote plan. It is safe to call when no remote plan has been installed.
-func (scan *Scan) closePlanIO() {
+func (scan *Scan) closePlanIO() error {
if scan.planIO == nil {
- return
+ return nil
}
planIO := scan.planIO
scan.planIO = nil
- planIO.releaseOwner()
+
+ return planIO.releaseOwner()
+}
+
+// Close releases the plan-scoped resources owned by this scan. It is safe to
+// call more than once. Active ReadTasks iterators retain their reader lease
and
Review Comment:
The active-iterator guarantee leaves an unreleasable edge: `ReadTasks`
increments the reader count before returning its lazy `iter.Seq2`, while
`releasePlanIOAfter` runs only if that sequence is invoked. If the caller
receives the iterator, never ranges over it, and calls `Scan.Close`, the owner
is released but the reader count remains permanently nonzero; `PlanIO.Close` is
never called, and `iter.Seq2` has no separate close operation. I reproduced
this with a focused test. Please defer acquisition until iteration starts or
otherwise provide deterministic abandonment cleanup.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]