Ensures that using a cursor after close has no effect.#803
Merged
Conversation
tgregg
force-pushed
the
uneventful-close
branch
from
9235587 to
fdf9ee5
Compare
popematt
reviewed
Apr 23, 2024
| /** | ||
| * Dummy state that indicates the cursor has been terminated and that additional API calls will have no effect. | ||
| */ | ||
| private static final RefillableState TERMINATED_STATE = new RefillableState(null, -1, -1); |
Contributor
There was a problem hiding this comment.
It seems sensible, even if it's not essential, for the TERMINATED_STATE to have a pre-populated state of TERMINATED.
Suggested change
| private static final RefillableState TERMINATED_STATE = new RefillableState(null, -1, -1); | |
| private static final RefillableState TERMINATED_STATE = new RefillableState(null, -1, -1); | |
| static { | |
| TERMINATED_STATE.state = State.TERMINATED; | |
| } |
| terminate(); | ||
| // Use a unified code path for all cursors after close. This path forces a termination check before accessing | ||
| // the input stream or buffer. | ||
| isSlowMode = true; |
Contributor
There was a problem hiding this comment.
Should isSlowMode = true be in terminate() so that it is also set on other conditions that result in a terminated state?
tgregg
force-pushed
the
uneventful-close
branch
from
fdf9ee5 to
bc40ac6
Compare
tgregg
force-pushed
the
uneventful-close
branch
from
bc40ac6 to
e408153
Compare
popematt
approved these changes
Apr 23, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes:
The
IonCursorinterface extendsCloseable, which includes documentation onclosethat states that callingcloseafter the resource is already closed has no effect. We do not define the behavior of the cursor when methods other thancloseare called after the cursor is closed, but I think it is reasonable for the cursor to remain in aNEEDS_DATAstate, indicating that it is not positioned on a value.Before the proposed changes, the added tests would fail with
NullPointerExceptionwhen the cursor attempted to access its buffer, which is set tonullduringclose. Entering theTERMINATEDstate in "slow" mode allows the cursor to make use of existing termination checks that gate all of the slow mode APIs.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.