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

use Interlocked.Increment to be thread safe #781

Merged
merged 1 commit into from Dec 12, 2015

Conversation

enricosada
Copy link
Contributor

changed newUnique and newStamp to be safe to use as shared global generators

@KevinRansom
Copy link
Member

Looks good to me. Nicely spotted too.

@@ -35,12 +35,12 @@ open Microsoft.FSharp.Core.CompilerServices
/// Unique name generator for stamps attached to lambdas and object expressions
type Unique = int64
//++GLOBAL MUTABLE STATE
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think now are thread safe, should i remove the //++GLOBAL MUTABLE STATE too? /cc @dsyme

Copy link
Contributor

Choose a reason for hiding this comment

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

Please change to //++GLOBAL MUTABLE STATE (thread safe)

@KevinRansom
Copy link
Member

It still is global mutable state, so the comment is not wrong.

@dsyme
Copy link
Contributor

dsyme commented Dec 10, 2015

AFAIK these are not called in a multi-threaded way in VFT. But for FCS this should indeed be locked

@enricosada
Copy link
Contributor Author

yes, i am trying to check all cache etc for thread safety, maybe we can unlock some parallelism in fsc

KevinRansom added a commit that referenced this pull request Dec 12, 2015
use Interlocked.Increment to be thread safe
@KevinRansom KevinRansom merged commit a504061 into dotnet:master Dec 12, 2015
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.

None yet

4 participants