Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FileChunkProducer.shouldProduceMore updates state #2888

Closed
wants to merge 1 commit into from

Conversation

rnro
Copy link
Contributor

@rnro rnro commented Sep 19, 2024

Motivation:

FileChunkProducer would not reflect its internal state when requested to produce more elements for the sequence after being paused.

Modifications:

FileChunkProducer.shouldProduceMore updates the internal state and returns the threadpool leading to more elements being produced.

Result:

FileChunkProducer should no longer hang after being paused.

Motivation:

`FileChunkProducer` would not reflect its internal state when requested to
produce more elements for the sequence after being paused.

Modifications:

`FileChunkProducer.shouldProduceMore` updates the internal state and
returns the threadpool leading to more elements being produced.

Result:

`FileChunkProducer` should no longer hang after being paused.
@rnro
Copy link
Contributor Author

rnro commented Sep 19, 2024

Should resolve #2887

@rnro rnro added the semver/patch No public API change. label Sep 19, 2024
@FranzBusch FranzBusch enabled auto-merge (squash) September 19, 2024 08:04
@finagolfin
Copy link
Contributor

This fixes the test hangs, but I now see randomly failing tests, particularly when run in parallel:

> ../swift-6.0-RELEASE-fedora39/usr/bin/swift test --filter FileHandleTests  --parallel
Building for debugging...                                                                                                                                          [1/1] Write swift-version-649633EA72C13463.txt                                                                                                                     Build complete! (0.22s)                                                                                                                                            [63/63] Testing NIOFileSystemIntegrationTests.FileHandleTests/testWriteSequenceOfBytes
Test Suite 'Selected tests' started at 2024-09-19 14:54:16.337
Test Suite 'FileHandleTests' started at 2024-09-19 14:54:16.338
Test Case 'FileHandleTests.testReadEndOffsetExceedsEOFClosedrange' started at 2024-09-19 14:54:16.338
/home/finagolfin/swift-nio/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift:492: error: FileHandleTests.testReadEndOffsetExceedsEOFClosedrange : XCTAssertEqual failed: ("[2f2f3d3d3d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d...6f6e732077652075736520666f722074656d706f726172792066696c65732e0a](6784 bytes)") is not equal to ("[2f2f3d3d3d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d...722e636f64652c202e636c6f736564290a202020207d0a7d0a23656e6469660a](54324 bytes)") -
Test Case 'FileHandleTests.testReadEndOffsetExceedsEOFClosedrange' failed (0.004 seconds)
Test Suite 'FileHandleTests' failed at 2024-09-19 14:54:16.343
         Executed 1 test, with 1 failure (0 unexpected) in 0.004 (0.004) seconds                                                                                   Test Suite 'Selected tests' failed at 2024-09-19 14:54:16.343
         Executed 1 test, with 1 failure (0 unexpected) in 0.004 (0.004) seconds
                                                                                                                                                                   Test Suite 'Selected tests' started at 2024-09-19 14:54:16.344
Test Suite 'FileHandleTests' started at 2024-09-19 14:54:16.345
Test Case 'FileHandleTests.testReadEndOffsetExceedsEOFPartialThrough' started at 2024-09-19 14:54:16.345
/home/finagolfin/swift-nio/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift:492: error: FileHandleTests.testReadEndOffsetExceedsEOFPartialThrough : XCTAssertEqual failed: ("[2f2f3d3d3d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d...696f6e54696d652c2046696c65496e666f2e54696d6573706563287365636f6e](51072 bytes)") is not equal to ("[2f2f3d3d3d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d...722e636f64652c202e636c6f736564290a202020207d0a7d0a23656e6469660a](54324 bytes)") -                                                                                                                                                         Test Case 'FileHandleTests.testReadEndOffsetExceedsEOFPartialThrough' failed (0.016 seconds)
Test Suite 'FileHandleTests' failed at 2024-09-19 14:54:16.361
         Executed 1 test, with 1 failure (0 unexpected) in 0.016 (0.016) seconds                                                                                   Test Suite 'Selected tests' failed at 2024-09-19 14:54:16.361
         Executed 1 test, with 1 failure (0 unexpected) in 0.016 (0.016) seconds

Test Suite 'Selected tests' started at 2024-09-19 14:54:16.356                                                                                                     Test Suite 'FileHandleTests' started at 2024-09-19 14:54:16.357
Test Case 'FileHandleTests.testReadEndOffsetExceedsEOFPartialUpTo' started at 2024-09-19 14:54:16.357
/home/finagolfin/swift-nio/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift:492: error: FileHandleTests.testReadEndOffsetExceedsEOFPartialUpTo : XCTAssertEqual failed: ("[2f2f3d3d3d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d...72794469726563746f72792874656d706c6174653a2022746573742d58585822](12800 bytes)") is not equal to ("[2f2f3d3d3d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d...722e636f64652c202e636c6f736564290a202020207d0a7d0a23656e6469660a](54324 bytes)") -
Test Case 'FileHandleTests.testReadEndOffsetExceedsEOFPartialUpTo' failed (0.006 seconds)
Test Suite 'FileHandleTests' failed at 2024-09-19 14:54:16.363                                                                                                              Executed 1 test, with 1 failure (0 unexpected) in 0.006 (0.006) seconds                                                                                   Test Suite 'Selected tests' failed at 2024-09-19 14:54:16.363
         Executed 1 test, with 1 failure (0 unexpected) in 0.006 (0.006) seconds

@rnro
Copy link
Contributor Author

rnro commented Sep 20, 2024

Hi @finagolfin thanks for following up on this, I was seeing this too in my testing. I've proposed that we revert the change for now and circle-back #2892.

@rnro rnro closed this Sep 20, 2024
auto-merge was automatically disabled September 20, 2024 12:16

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants