From ae851d92f7c6a0aae11290d8f8d3a9bd8369670c Mon Sep 17 00:00:00 2001 From: davidby-influx <72418212+davidby-influx@users.noreply.github.com> Date: Tue, 15 Jun 2021 12:45:34 -0700 Subject: [PATCH] fix: do not panic on cleaning up failed iterators (#21666) (#21697) We have seen occasional panics in Iterators.Close() when cleaning up after failed iterator creation. This commit checks for nil on any iterator to be closed, and now returns any errors generated by that Close(). Closes https://github.com/influxdata/influxdb/issues/19579 Closes https://github.com/influxdata/influxdb/issues/19476 (cherry picked from commit acc4105b8c48657287ddeaed56c28fd2840dd862) Closes #21667 --- CHANGELOG.md | 3 ++- query/iterator.go | 17 ++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dcb62684fdf..917117f0640 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,10 @@ -[unreleased] +v1.9.3 [unreleased] ### Bugfixes - [#21609](https://github.com/influxdata/influxdb/pull/21609): fix: avoid rewriting fields.idx unnecessarily - [#21695](https://github.com/influxdata/influxdb/pull/21695): fix: Do not close connection twice in DigestWithOptions +- [#21697](https://github.com/influxdata/influxdb/pull/21697): fix: do not panic on cleaning up failed iterators v1.9.2 [unreleased] diff --git a/query/iterator.go b/query/iterator.go index 81ff89f93af..95064390451 100644 --- a/query/iterator.go +++ b/query/iterator.go @@ -43,11 +43,22 @@ func (a Iterators) Stats() IteratorStats { } // Close closes all iterators. -func (a Iterators) Close() error { +// We are seeing an occasional panic in this function +// which looks like a nil reference from one +// itr.Close() call, thus we check for nil elements +// in the slice a. This is often called as error +// clean-up, so the state of the iterators may be +// unhappy. +func (a Iterators) Close() (err error) { + err = nil for _, itr := range a { - itr.Close() + if itr != nil { + if e := itr.Close(); e != nil && err == nil { + err = e + } + } } - return nil + return err } // filterNonNil returns a slice of iterators that removes all nil iterators.