Runtime: Investigate all Double-Check locked patterns and consider static or Lazy<T>

[CIPop]

Investigate all Double-Check locked patterns and consider alternative singleton implementations such as the static or Lazy<T> patterns.

See: http://csharpindepth.com/Articles/General/Singleton.aspx

[omariom]

I have one for you https://github.com/dotnet/coreclr/issues/1157 :)

[CIPop]

Thanks @omariom!

From dotnet/coreclr#1157 :

Using the System.Lazy pattern (which doesn't use volatile) would also be an improvement in performance over the existing code, especially on ARM. The advantage of Lazy over static field initializer is that it supports lazy semantics (create object on first read) but since the overhead of creating an EqualityComparer is low I prefer the static field initializer option.

[karelz]

Next steps: We need sweep of code base and changes to it.
Warning: Be careful about changing semantics (some of them might not be easy).
Complexity: Medium.

[WinCPP]

@karelz The other one on my name is not moving yet. Can I take this instead? Sounds interesting.

[karelz]

Sure assigning to you ...
I would recommend to start with the easier, straightforward cases. Maybe even do just a couple in initial PR to check first round of review feedback. Either way, breaking up cross-cutting work into not-so-big PRs is always good.

[WinCPP]

@karelz Thanks for the direction. I'm on this now!

[WinCPP]

@karelz @CIPop @stephentoub In the above PR, I have targeted the 'static volatile' fields. Code has double-check locked initialisation for instance fields, as well. Would it be correct to say that those are not of concern? Appreciate your inputs.

[WinCPP]

@stephentoub @karelz The PR was for subset of files. There are more to come. We need to keep this issue open... Thanks!

[stephentoub]

@WinCPP, it was closed automatically by GitHub based on the text of the PR you submitted (it included the text "Fix dotnet/corefx#3862"). For subsequent PRs, don't include a statement that it fixes the issue, and it won't be closed automatically :)

[WinCPP]

@stephentoub yup I got it :)

[WinCPP]

@stephentoub There are multiple _instance field_ initializations which make use of double-check locked pattern and which could be perhaps replaced with LazyInitializer.EnsureInitialize.

For example, this place could make use of locked based overload, not a singleton initialization in true sense (ref. opening comment in this issue). Appreciate your thoughts.

[stephentoub]

There's nothing special about statics in this regard; it applies to instance fields as well.

[WinCPP]

Just making a note... dotnet/corefx#16274 could be used for this work once the new overload is available.

[WinCPP]

@stephentoub I have an implementation of dotnet/corefx#16274 in a branch on my fork of coreclr repository here. Was not sure if I can contribute over there, but if that API came through early, it would have helped in this issue. Hence ...

I believe following is the process:

• [x] Implement the API in coreclr repository
• [ ] Augment test cases (I don't find any around LazyInitializer... I think I am looking in wrong place?)
• [ ] Expose the API in corefx (not sure how that works)

Appreciate your inputs. Thanks!

[karelz]

@WinCPP let's do the dotnet/corefx#16274 implementation separately in that issue. If you want to block this work on it, it is fine, but all details regarding its implementation should go into dotnet/corefx#16274. Let's not mix issues.

[WinCPP]

@karelz Sure. I will use dotnet/corefx#16274. Moving the discussion there. Yes, I wanted to park this issue and work on the other API. And then use it in this issue. That would eliminate another pass through the code to do the replacements. The new API by @stephentoub is of great use for this current issue.

[WinCPP]

@karelz @stephentoub Two questions,

a. Files such as corefxsrcCommonsrcSystemNetSocketProtocolSupportPal.Unix.cs appear to be platform specific. To make changes to these files, compile and test them, I think I would need to setup some Unix box with the entire tool chain...? Or should I directly rely on the CI pipelines? This is different territory for me. Kindly advise.

b. Some of the projects e.g. srcSystem.ComponentModel.AnnotationssrcSystem.ComponentModel.Annotations.csproj have just 'mscorlib' in references. For these, LazyInitializer.EnsureInitialized resolves (in VS IDE) to netstandard.dll (following is the header in the LazyInitializer.cs file generate from metadata). And this doesn't have the new LazyInitializer.EnsureInitialized overload.
```C#

region Assembly netstandard, Version=2.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51

// M:corefxbinrefnetstandardnetstandard.dll

endregion

`` So should the references for such projects be changed to directly refer tocorefxbinrefnetcoreappSystem.Threading.dll` via the _project reference_ src on similar lines as in srcSystem.Private.XmlsrcSystem.Private.Xml.csproj? Also, whatever I try, it keeps going back to netstandard, even if I remove mscorlib and explicitly add the netcoreapp ref mentioned previously.

[stephentoub]

I think I would need to setup some Unix box with the entire tool chain...?

Ideally, yes.

Or should I directly rely on the CI pipelines?

If you're unable to set up a Linux VM for some reason, then this is an ok fallback.

And this doesn't have the new LazyInitializer.EnsureInitialized overload.

If in some projects you're currently unable to use the new overload, just skip those cases.

[WinCPP]

@stephentoub @karelz Had Ubuntu in dual boot config on my machine... After arduous upgrade to 16.x, I was able to build corefx on ubuntu. But now I have got two copies of code... on Win 10 and on Ubuntu. Too much of pull push across boots via remote on fork.

Also tried accessing Windows drives in Ubuntu, compilation went through to some extent but then failed saying that project.json doesn't have _ubuntu_ configuration. Gave up on that :(. Any tips on how to share code between dual boot? Next step, gonna try Ubuntu on Hyper-V on Win 10. But I think I will end up with same project.json issue as it will still be _independent_ ubuntu environment? Appreciate any tips. Err... should I be asking such queries to some other forum...

[stephentoub]

Any tips on how to share code between dual boot?

I don't have advice on that as I typically don't dual boot; I typically use Linux VMs, either in Azure or locally with Hyper-V or VirtualBox.

[WinCPP]

Hmm. I was actually looking for way to cross-compile Unix / Uap related files on Windows, if and when required; which I found later in the "build the repository" document...

[WinCPP]

@stephentoub Please refer to this location which uses double-check locked execution of some code for initializing multiple variables together. Would having a method EnsureExecuted on lines of LazyInitializer.EnsureInitialized be worth for such types cases? The method could have some following prototype. Please refer to sample implementation code that I put up here.

C# public static bool EnsureExecuted(ref bool executed, ref object syncLock, Func<bool> delayedExecutor)
Just asking this out of curiosity. Also I don't know if there is some other forum where I should actually put this query, hence asking here, although it is not related to this specific issue.

Thanks,
Mandar

[stephentoub]

I suggest just leaving that one as is.

[WinCPP]

Yup.

Just for documentation purpose, this comment expresses potential to use lazy initialization, but being single-call-multi-variable initialization, it can be done only if there were EnsureExecuted or equivalent construct.