Skip to content

[Dependency Scanning] Guard access to shared state of the DependencyScannerDiagnosticCollectingConsumer #71792

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
Feb 22, 2024

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Feb 21, 2024

DependencyScanningTool has one DependencyScannerDiagnosticCollectingConsumer which gets associated with each individual scan query during lifetime of the tool. This isn't a valid use of such diagnostic consumer because the tool may be handling multiple scan queries at any given time, which may all be adding/resetting/querying diagnostics. For now, guard the shared state of the shared consumer as a short-term fix.

A follow-up change will refactor the diagnostic consumer to be per-client-scan.

Resolves rdar://123344065

@artemcm
Copy link
Contributor Author

artemcm commented Feb 21, 2024

@swift-ci smoke test

Copy link
Contributor

@cachemeifyoucan cachemeifyoucan left a comment

Choose a reason for hiding this comment

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

convertDiagnosticInfo seems an unused declaration that can be removed.

Would be good to write a test. I added swift-scan-test tool that can be used to call into libSwiftScan APIs.

@artemcm artemcm enabled auto-merge February 21, 2024 20:53
…ScannerDiagnosticCollectingConsumer`

Resolves rdar://123344065
@artemcm artemcm force-pushed the DepScanDiagConsumerIsolate branch from c87a4d7 to 5c4bbc1 Compare February 21, 2024 22:59
@artemcm artemcm disabled auto-merge February 21, 2024 22:59
@artemcm
Copy link
Contributor Author

artemcm commented Feb 21, 2024

@swift-ci smoke test

@artemcm
Copy link
Contributor Author

artemcm commented Feb 22, 2024

@swift-ci smoke test Windows platform

@artemcm
Copy link
Contributor Author

artemcm commented Mar 1, 2024

Longer-term fix landed in #71907

@artemcm artemcm deleted the DepScanDiagConsumerIsolate branch March 1, 2024 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants