Fix concurrency issue regression #47
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When code has been moved from targets to csharp in commit c3354fa file read has been excluded from the mutex which produce concurrency issue like explain in the comment above the mutex creation - Reduce scope of the mutex to optimize parallel compilations This is a really interesting proposition from issue baseclass#36
…he file Mutex constructor search for the file when we provide a full path as a mutex name...
|
Thanks for the pull request. Why did you remove the existence of the file before creating the mutex? There is no need to create it when the file is already there. |
|
The file can be there but not fully written by one execution which unzip it. So expecting we can read the file just because it is present is wrong. |
jmarsik
added a commit
to lekiscz/Contrib.Nuget
that referenced
this pull request
Jan 3, 2023
…ith merged PR baseclass#47, version number corresponds to base version from upstream repo
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We recently upgrade Baseclass.Contrib.Nuget.Output package to support VS2017
However we discover that recent versions have a concurrency issue similar to the one I had fixed in the past.
Looking at the code it appears that a regression appear when the c# code has been moved from the targets file to a cs file in the c# project.
The mutex do not cover any more the read file operation but only the file extraction operation which cause concurrency issue when one try to read the file while one is currently extracting it which sometimes occurs on our build servers in CI.
The intend of this pull request is to fix that regression by re-introduce the file read operation inside the mutex.
I've also include the idea of issue #36 to improve performances in a concurrent build environment.
This is now running fine on our build servers with an internal pre-release package.