-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
String(format:)
intermittently returns an empty string when run concurrently by swift-testing
#5152
Comments
euanh
changed the title
Dec 20, 2024
String(format:)
fails intermittently when run concurrently by swift-testing
String(format:)
intermittently returns an empty string when run concurrently by swift-testing
euanh
added a commit
to euanh/swift-container-plugin
that referenced
this issue
Dec 20, 2024
String(format:) sometimes returns an empty string even if the number being formatted can be represented in an octal string of the requested length. This may be a thread-safety problem and has only been seen when running the tests. swiftlang/swift-corelibs-foundation#5152 This commit changes octal6/11 to use String(value, radix:) and handle padding directly. This has not failed during hundreds of test runs. Benchmarking the old and new implementations of octal6() shows that the new version is about twice as fast and makes no allocations, whereas the old version made 7 allocations.
euanh
added a commit
to euanh/swift-container-plugin
that referenced
this issue
Dec 20, 2024
String(format:) sometimes returns an empty string even if the number being formatted can be represented in an octal string of the requested length. This may be a thread-safety problem and has only been seen when running the tests. swiftlang/swift-corelibs-foundation#5152 This commit changes octal6/11 to use String(value, radix:) and handle padding directly. This has not failed during hundreds of test runs. Benchmarking the old and new implementations of octal6() shows that the new version also is about twice as fast as the old one and makes no allocations, whereas the old version made 7 allocations.
euanh
added a commit
to euanh/swift-container-plugin
that referenced
this issue
Jan 6, 2025
String(format:) sometimes returns an empty string even if the number being formatted can be represented in an octal string of the requested length. This may be a thread-safety problem and has only been seen when running the tests. swiftlang/swift-corelibs-foundation#5152 This commit changes octal6/11 to use String(value, radix:) and handle padding directly. This has not failed during hundreds of test runs. Benchmarking the old and new implementations of octal6() shows that the new version also is about twice as fast as the old one and makes no allocations, whereas the old version made 7 allocations.
euanh
added a commit
to euanh/swift-container-plugin
that referenced
this issue
Jan 6, 2025
String(format:) sometimes returns an empty string even if the number being formatted can be represented in an octal string of the requested length. This may be a thread-safety problem and has only been seen when running the tests. swiftlang/swift-corelibs-foundation#5152 This commit changes octal6/11 to use String(value, radix:) and handle padding directly. This has not failed during hundreds of test runs. Benchmarking the old and new implementations of octal6() shows that the new version also is about twice as fast as the old one and makes no allocations, whereas the old version made 7 allocations.
euanh
added a commit
to apple/swift-container-plugin
that referenced
this issue
Jan 6, 2025
Motivation ---------- The `tar` writer is tested by the end to end tests, but unit tests are more helpful for refactoring and extending it. Modifications ------------- * Extract `tar` into its own module * Extract tar header construction into a separate function * Add initial unit tests for the helper methods used to build `tar` headers * Stop using `Swift(format:)` to convert integers into octal strings because of swiftlang/swift-corelibs-foundation#5152 Result ------ The basic helper functions underpinning the tar writer will have unit tests, making it easier to test, refactor and extend the tar writer in future. Test Plan --------- This pull request adds new tests. All existing tests continue to pass.
@euanh thanks for reporting this! I've found the root cause of the issue and addressed it in this copy of CoreFoundation that ships with Swift on Linux/Windows at #5155. We'll make sure that the copy of CoreFoundation that ships with Darwin platforms is also updated with the fix so that platforms like macOS are corrected as well. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The following test, run from the command line in a loop, will eventually fail at random because the String constructor returns an empty string:
On my machine it fails roughly once in every 20 runs, on different test inputs. The error is always that the constructor returns an empty string. The inputs are fixed and always in a range which can be represented in a 6 character octal string.
I only noticed this when I started to use
swift-testing
, which runs test cases in parallel by default. It has never come up in actual use, as far as I'm aware, because I only use this constructor in single-threaded sequential code.Could there be a shared buffer somewhere in the
CFString
underpinnings which makes this constructor not threadsafe? I didn't see anything warning about this in the docs: https://developer.apple.com/documentation/swift/string/init(format:_:).XCode 16.2 / 16C5032a
macOS 15.1.1 (24B91)
% swift -version
swift-driver version: 1.115.1 Apple Swift version 6.0.3 (swiftlang-6.0.3.1.10 clang-1600.0.30.1)
Target: arm64-apple-macosx15.0
The text was updated successfully, but these errors were encountered: