Repository navigation
Conversation
…d FlowReactiveStreams
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.
Flow.fromPublisherbuffers elements in a channel. Some of them could be received from the publisher but never emitted, and nothing released them. That leaks elements which hold resources, such as Netty's ref-counted ByteBufs. Tapir currently tracks these elements itself (softwaremill/tapir#5573).fromPublisher(p, onDiscard: T => Unit), in core and inFlowReactiveStreams. Each element received from the publisher is either emitted, or passed toonDiscardexactly once. Elements are discarded when the flow fails, is interrupted, or stops early (e.g.take), and also when they arrive after the subscription is cancelled.onDiscardmay run on publisher threads and must not throw.take, and elements arriving after cancellation. Also added a stress test that checks each element is either emitted or discarded, never both or neither. Updateddoc/streaming/flows.md.Closes #528
Open review findings
What the run's review left open, each with the reason recorded — including findings the fixer declined:
git rev-parse --is-bare-repositoryprints false; the git dir is /home/adamw/ox/.git/worktrees/zippy-astrodon-94aa2fb6). The exception comes from the sbt-git plugin, which uses JGit by default and reads this linked worktree as bare while the build loads. Nothing in project/ or the build configures this;zz-tmp-git.sbtwas my temporary workaround, not the cause. To check the code, I recreated it withuseReadableConsoleGit, which makes sbt-git use command-line git:sbt core/Test/compile flowReactiveStreams/Test/compilesucceeded (warnings only, in older test files, no errors) and flowReactiveStreams tests passed. I then deleted the file again. Changing the shared build's git plugin setup is outside the scope of this change.sbt Test/compilefails with no diagnostic — Environmental. Full output shows sbt-git's JGit throws NoWorkTreeException ("Bare Repository has neither a working tree, nor an index") while loading the build, because this checkout is a git worktree; no source error is involved. With a temporaryThisBuild / git.gitUncommittedChanges := falseoverride,sbt Test/compilesucceeds across all modules (only pre-existing unrelated warnings). The temporary file was removed; no codebase change is needed.core/src/main/scala/ox/flow/FlowCompanionReactiveOps.scala:141) — A deterministic test can't be written. In jox 1.1.2,receiveOrClosedonly throwsInterruptedExceptionwhen it parks (Continuation.await). AfterdoneOrClosed()has finished on the draining thread, a receive never parks: buffered or in-flight sends are returned or retried, and later cells are already CLOSED. The catch is reached only in a narrow race, when a concurrentonComplete/onErroris still closing cells. I tried the suggested test (interrupt plus throw in the consumer, then assert the discards and the interrupt flag). It passed both with the catch removed and with the re-interrupt removed, so it did not cover the branch, and I reverted it. The suggested capacity-1 variant also doesn't help, because a blocked sender's Continuation is resumed without parking.NoWorkTreeException: Bare Repository has neither a working tree, nor an indexwhile evaluatinggitUncommittedChanges, so the build fails before compiling. It is not a code problem. With a temporaryzz-tmp-git.sbtthat setsThisBuild / git.gitUncommittedChanges := false, Test/compile and the core reactive and flowReactiveStreams tests all pass. I deleted the temporary file afterwards.sbt Test/compileexits 1 with no compiler diagnostics — Environmental, not a code problem. In this git worktree, sbt-git's JGit throwsNoWorkTreeException: Bare Repository has neither a working tree, nor an indexwhile evaluatinggitUncommittedChangesduring build load. The build fails before any compilation, and the full output shows that stack trace above the timing line. With a temporaryzz-tmp-git.sbtsettingThisBuild / git.gitUncommittedChanges := false,Test/compilesucceeds. With the same file,core/testOnly ox.flow.reactive.*(44 passed, 7 ignored) andflowReactiveStreams/test(2 passed) also pass. I deleted the file afterwards, so it is not left in the tree.