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

ProjectImported OM should only exist in logger thread context #10960

Closed
JanKrivanek opened this issue Nov 8, 2024 · 0 comments · Fixed by #11049
Closed

ProjectImported OM should only exist in logger thread context #10960

JanKrivanek opened this issue Nov 8, 2024 · 0 comments · Fixed by #11049
Assignees
Labels
Priority:2 Work that is important, but not critical for the release triaged

Comments

@JanKrivanek
Copy link
Member

JanKrivanek commented Nov 8, 2024

Context

#10959 (comment)

The _deferredProjectEvalIdToImportedProjects should be ensured to be called only from the BuildEventArgs handlers - so in a synchronized context. Then we can gt rid of any synchronization on it

What needs to be done

The handler of BuildCheckBuildEventHandler.HandleProjectEvaluationStartedEvent and ProcessProjectEvaluationStarted needs to be separated, as currently we cannot distinguish the situation "ProjectEvaluationStartedEvent occured, but it was received through logging infrastructure".

The reason for the 2 possible sources of the event (through the logging or directly from engine) is due to the fact that the BuildCheckManager can live in the worker node as well.

The ProjectImported data are to be received only via the logging infrastructure - so it should be handled only in the handler of BuildCheckBuildEventHandler.HandleProjectEvaluationStartedEvent. Since the logging infra guarantees synchronized (single threaded) delivery of events - we can then be sure that all handling of the ProjectImported data happens without concurrency risks and any synchronization (or usage of ConcurrentDictionary) can be removed. This should significantly simplify the code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:2 Work that is important, but not critical for the release triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants