Consolidate duplicated read() error handling and add spring-core interrupt test for SubscriberInputStream - #4
Draft
jaipilot[bot] wants to merge 1 commit into
Conversation
…rrupt test for SubscriberInputStream
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
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.
Context
The PR fixes a busy-spin/interrupt-handling bug in the lock-free await() loop shared (as documented near-duplicates) by
spring-web'sorg.springframework.http.client.SubscriberInputStreamandspring-core'sorg.springframework.core.io.buffer.SubscriberInputStream, and rethrowsIOExceptionas-is from bothread()andread(byte[], int, int)instead of wrapping it viaExceptions.propagate.What changed here
spring-core'sSubscriberInputStreamhad zero direct test coverage of the interrupt-while-awaiting-data fix (onlyspring-web's sibling class got a test in the PR). AddedDataBufferUtilsTests.inputStreamSubscriberInterruptWhileAwaitingData(), mirroring the existingspring-webtest against the reactivestreams-basedDataBufferUtils.subscriberInputStream()API: a publisher that never emits, a reader thread that parks inread(), aninterrupt()call, and assertions that the read returns viaInterruptedIOException("Interrupted while awaiting data")with the interrupt status restored.SubscriberInputStreamclasses, the PR's new 3-line IOException-rethrow snippet was appended identically to thecatch (Throwable ex)blocks of bothread()andread(byte[], int, int), which already had an identical remainder (this.closed = true; requiredSubscriber().cancel(); cleanAndFinalize(); throw Exceptions.propagate(ex);). The two catch blocks were now fully identical duplicates within each class. Extracted a privatehandleReadError(Throwable ex) throws IOExceptionhelper per class and replaced both catch bodies withthrow handleReadError(ex);. No change to thrown exception types, messages, ordering ofclosed/cancel/cleanAndFinalize, or the interrupt-status contract; both classes remain package-private andfinalwith no subclassing/reflection use, so there is no external-compatibility impact.Evidence
behavior_baselineandbehavior_candidate: identical focused command (:spring-core:test --tests DataBufferUtilsTests+:spring-web:test --tests SubscriberInputStreamTests) passed before and after the cleanup edit.full_build::spring-core:checkand:spring-web:check(compile for all Java release variants, Checkstyle, architecture check, JMH compile, full test suites) passed.Not changed
await()/parkedThreadCAS-park-unpark protocol itself was left untouched: it is delicate, correctness-critical concurrency code, and no algorithmic or allocation improvement in the PR's changed lines could be justified with the required timing/operation-count evidence for what is a cold (interrupt/spurious-wakeup only) path.Generated by JAIPilot Cloud for #3 from Anthropic session
sesn_01Kf9h5j18YVjm6Y2cRZ77pf.