runtime: Bug in CultureInfo.Clone() causes .Calendar and .DateTimeFormat.Calendar to diverge

Courtesy of excellent sleuthing by @lpatalas:

Accessing a CultureInfo instance’s .DateTimeFormat property before it is cloned with .Clone() unexpectedly causes the clone’s .Calendar and .DateTimeFormat.Calendar properties to reference different objects; from https://github.com/PowerShell/PowerShell/issues/10438#issuecomment-529628273:

If I would have to guess it’s caused by this line: https://github.com/dotnet/coreclr/blob/master/src/System.Private.CoreLib/shared/System/Globalization/CultureInfo.cs#L1015. The _calendar field is null so it’s not cloned until it’s accessed for the first time. After that it’s set and each subsequent Clone() call will create new Calendar instance.

Code to reproduce:

using System;
using System.Globalization;

public static class Program
{
    public static void Main()
    {
      var orig = CultureInfo.InvariantCulture;
      
      for (var i=0; i<2; ++i) {

        var clone = (CultureInfo)orig.Clone();
        clone.Calendar.TwoDigitYearMax = 2020;

        Console.WriteLine($"Clone {i}: Do .Calendar and .DateTimeFormat.Calendar referrence the same object? {object.ReferenceEquals(clone.Calendar, clone.DateTimeFormat.Calendar)}");
        Console.WriteLine($"Clone {i}: .Calendar.TwoDigitYearMax vs. .DateTimeFormat.Calendar.TwoDigitYearMax: {clone.Calendar.TwoDigitYearMax} vs. {clone.DateTimeFormat.Calendar.TwoDigitYearMax}");

        // Trigger the bug: after this property access,
        // cloning `orig` again makes the clones' .Calendar and .
        // DateTimeFormat.Calendar references *differ*.
        var dummy = orig.DateTimeFormat;
      }

    }
}

Expected:

Clone 0: Do .Calendar and .DateTimeFormat.Calendar referrence the same object? True
Clone 0: .Calendar.TwoDigitYearMax vs. .DateTimeFormat.Calendar.TwoDigitYearMax: 2020 vs. 2020
Clone 1: Do .Calendar and .DateTimeFormat.Calendar referrence the same object? True
Clone 1: .Calendar.TwoDigitYearMax vs. .DateTimeFormat.Calendar.TwoDigitYearMax: 2020 vs. 2020

Actual (the 2nd iteration exhibits the bug):

Clone 0: Do .Calendar and .DateTimeFormat.Calendar referrence the same object? True
Clone 0: .Calendar.TwoDigitYearMax vs. .DateTimeFormat.Calendar.TwoDigitYearMax: 2020 vs. 2020
Clone 1: Do .Calendar and .DateTimeFormat.Calendar referrence the same object? False
Clone 1: .Calendar.TwoDigitYearMax vs. .DateTimeFormat.Calendar.TwoDigitYearMax: 2020 vs. 2029

About this issue

Original URL
State: closed
Created 5 years ago
Reactions: 1
Comments: 19 (9 by maintainers)

Most upvoted comments

I think the core of the issue is that the getter of CultureInfo.DateTimeFormat changes state in ways that are observable. It’s not just lazy initialization or caching. It’s actually a side-effect in a property getter.

GSPP on Sep 11, 2019

Thanks @mklement0 for reporting and discussing this issue.

tarekgh on Sep 16, 2019

@mklement0 thanks. I’ll think about it and will try to find if there is a way we can do it without allocating more objects.

tarekgh on Sep 11, 2019

Reopening to continue discussion above.

danmoseley on Sep 11, 2019

while changing the state of this object between the 2 cloning operations

There is no intentional change of state here - just an implementation detail (lazy initialization) that should never affect the behavior.

In fact, in PowerShell you don’t get a choice: merely referencing [cultureinfo]::InvariantCulture] retrieves the .DateTimeFormat property behind the scenes and therefore invariably surfaces the problem.

CultureInfo.Clanedar and CultureInfo.DateTimeFormat.Calendar is not necessary to match all the time.

What is their intended relationship and when does it ever make sense for them to point to different objects? It seems to me that .Calendar, as a read-only property, is only ever meant to reflect the same value as the read-write .DateTimeFormat.Calendar property; from the documentation of the .Calendar property:

Your application changes the calendar used by the current CultureInfo by setting the Calendar property of DateTimeFormat

If the current behavior is by design, I suggest revisiting that design, given its highly obscure and hard-to-predict-by-the-user nature. Please reopen this issue.

mklement0 on Sep 11, 2019

Actually this is by design. here is some comments why:

Cloning depends on the object state. If you cloned the same object twice while changing the state of this object between the 2 cloning operations, then expect you get a different cloned objects.
When requesting CultureInfo.DateTimeFormat for the first time, it is actually changing the cultureInfo object state as it creates a new DateTimFormatInfo object using the CultureInfo.Calendar object.
CultureInfo.Clanedar and CultureInfo.DateTimeFormat.Calendar is not necessary to match all the time. If you tweak one it doesn’t have to affect the other. And it depends if you tweak CultureInfo.Calendar before of after CultureInfo.DateTimeFormat get created. As I mentioned first time you request CultureInfo.DateTimeFormat, will create the DateTimeFormatInfo using CultureInfo.Calendar.
During the CultureInfo cloning we don’t create a new objects for the properties which has null values for the following reason:
- It is not good to change the original object state while we are just cloning it.
- The most important reason is the perf. To avoid creating more objects when cloning while not necessary these objects are going to be used.

I agree this maybe a little bit not clear that when requesting CultureInfo.DateTimeFormat is changing the CultureInfo. We can clarify that in the docs if needed.

tarekgh on Sep 10, 2019

Actually accessing CultureInfo.DateTimeFormat alone is enough to trigger behaviour described above.

lpatalas on Sep 9, 2019