Skip to content

fix silent crash when output from compiler is too large #3478

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

Merged
merged 1 commit into from
May 7, 2021

Conversation

tomerd
Copy link
Contributor

@tomerd tomerd commented May 7, 2021

motivation: silent crashes, confused uers

changes:

  • change the order in swiftCompilerOutputParser such that print the stdout happens before any parsing attenpts are made
  • constraint the parsing attempts to output < 10k, since otherwise they struggle to complete

motivation: silent crashes, confused uers

changes:
* change the order in swiftCompilerOutputParser such that print the stdout happens before any parsing attenpts are made
* contraint the parsing attempts to output < 10k, since otherwise they struggle to complete

rdar://77558631
@tomerd
Copy link
Contributor Author

tomerd commented May 7, 2021

@swift-ci please smoke test

@tomerd tomerd added the ready Author believes the PR is ready to be merged & any feedback has been addressed label May 7, 2021
@tomerd
Copy link
Contributor Author

tomerd commented May 7, 2021

00:58:45 Test Case '-[PackageCollectionsTests.PackageCollectionsTests testUpdateAuthTokens]' started.
00:58:45 Exited with signal code 11
00:58:45 Build step 'Execute shell' mar

cc @yim-lee seems like some race condition there? code looks fine tbh and TSan also happy 🤔

@tomerd
Copy link
Contributor Author

tomerd commented May 7, 2021

@swift-ci please smoke test macOS

yim-lee added a commit to yim-lee/swift-package-manager that referenced this pull request May 7, 2021
Motivation:
Test crashed in CI: swiftlang#3478 (comment)

Modification:
Use `ThreadSafeKeyValueStore` to as backing store of `authTokens`.
@yim-lee
Copy link
Contributor

yim-lee commented May 7, 2021

seems like some race condition there?

I suppose it doesn't hurt to use ThreadSafeKeyValueStore instead: #3479

Copy link
Contributor

@abertelrud abertelrud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

yim-lee added a commit that referenced this pull request May 7, 2021
Motivation:
Test crashed in CI: #3478 (comment)

Modification:
Use `ThreadSafeKeyValueStore` to as backing store of `authTokens`.
yim-lee added a commit to yim-lee/swift-package-manager that referenced this pull request May 7, 2021
Motivation:
Test crashed in CI: swiftlang#3478 (comment)

Modification:
Use `ThreadSafeKeyValueStore` to as backing store of `authTokens`.
@tomerd tomerd merged commit caf6d54 into swiftlang:main May 7, 2021
tomerd added a commit to tomerd/swift-package-manager that referenced this pull request May 7, 2021
motivation: silent crashes, confused uers

changes:
* change the order in swiftCompilerOutputParser such that print the stdout happens before any parsing attenpts are made
* contraint the parsing attempts to output < 10k, since otherwise they struggle to complete

rdar://77558631
tomerd added a commit to tomerd/swift-package-manager that referenced this pull request May 7, 2021
motivation: silent crashes, confused uers

changes:
* change the order in swiftCompilerOutputParser such that print the stdout happens before any parsing attenpts are made
* contraint the parsing attempts to output < 10k, since otherwise they struggle to complete

rdar://77558631
tomerd pushed a commit that referenced this pull request May 10, 2021
Motivation:
Test crashed in CI: #3478 (comment)

Modification:
Use `ThreadSafeKeyValueStore` to as backing store of `authTokens`.
bitjammer pushed a commit to bitjammer/swift-package-manager that referenced this pull request Jul 23, 2021
Motivation:
Test crashed in CI: swiftlang#3478 (comment)

Modification:
Use `ThreadSafeKeyValueStore` to as backing store of `authTokens`.
bitjammer pushed a commit to bitjammer/swift-package-manager that referenced this pull request Jul 23, 2021
motivation: silent crashes, confused uers

changes:
* change the order in swiftCompilerOutputParser such that print the stdout happens before any parsing attenpts are made
* contraint the parsing attempts to output < 10k, since otherwise they struggle to complete

rdar://77558631
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Author believes the PR is ready to be merged & any feedback has been addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants