[Dependency Scanning] Give each scanner worker a unique Diagnostic Engine

[artemcm]

Otherwise, when multiple workers encounter a diagnostic simultaneously we can encounter races which lead to corrupted diagnostic data or crashes

Resolves rdar://159598539

[cachemeifyoucan]

I guess the reason for this is because there is state preserving variable in DiagnosticsEngine that affects behavior? But this only pushing the problem downstream, requires DiagnosticConsumer also having the behavior.

It is also generally required that Consumer.finishProcessing() only gets called once, so multiple diagnostics engine owning the same consumer is a bit asking for trouble. I like the idea, but I don't think this change actually fixes the problem.

[artemcm]

I guess the reason for this is because there is state preserving variable in DiagnosticsEngine that affects behavior? But this only pushing the problem downstream, requires DiagnosticConsumer also having the behavior.

It is also generally required that Consumer.finishProcessing() only gets called once, so multiple diagnostics engine owning the same consumer is a bit asking for trouble. I like the idea, but I don't think this change actually fixes the problem.

The only consumer the scanner's diagnostic engine should have is the DependencyScanDiagnosticCollector, which is thread-safe with respect to adding new diagnostics, but I am taking a look at how we handle finishProcessing.

[cachemeifyoucan]

The only consumer the scanner's diagnostic engine should have is the DependencyScanDiagnosticCollector

Maybe it is better if the worker consumer is created and added where the diagnostics engine is created? Also add comments about the requirement for all consumers to be threadSafe. Even better is create a ThreadSafeDiagnosticConsumer sub-class that has finishProcess() final that does nothing (or put the lock into this class).

[artemcm]

Maybe it is better if the worker consumer is created and added where the diagnostics engine is created?

It kind of already is. In DependencyScanningTool::getDependencies we create the collector consumer immediately before creating the scanner Compiler Instance and add it to its implicitly-created diagnostic engine.

I added a pure virtual ThreadSafeDiagnosticCollector which owns the lock and has a final do-nothing finishProcessing. I also added a stress-test unit test which reliably hits the race and corruption that this PR addresses.

[artemcm]

@swift-ci smoke test

[cachemeifyoucan]

Nit: I don't know if there is a naming convention in Swift code, but in LLVM, this usually named handleDiagnosticImpl.

[artemcm]

@swift-ci smoke test

[artemcm]

@swift-ci smoke test macOS platform

[artemcm]

@swift-ci smoke test

[artemcm]

@swift-ci smoke test Windows platform