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

System.FieldAccessException: Cannot set initonly static field #104242

Closed
LunarWhisper opened this issue Jul 1, 2024 · 9 comments
Closed

System.FieldAccessException: Cannot set initonly static field #104242

LunarWhisper opened this issue Jul 1, 2024 · 9 comments

Comments

@LunarWhisper
Copy link

Description

The original issue:
#11571
dotnet/standard#1598

We faced the same issue when migrating from .NET Framework to .NET8.

Sorry, but this is a regression bug. I kindly ask you to provide the ability to disable optimizations and return the previous behavior to the ability to override static initonly fields.

You can provide any alternative, even a config for the built-in detours, but we need such an opportunity while .NET and third-party libraries use static initonly fields and properties.

Use cases:

  1. Overriding the default behavior of .NET and 3rd-party types built around the use of static APIs such as ThreaPool. For example, we use this to impose an end-to-end limit on how many Parallel.ForEachs can run at the same time. At any time, you can carry out a refactoring that will make the readonly field we need and this technique will stop working.

  2. Mock dependencies in tests. For example, we use a third-party library that provides an interface for interacting with DNS via a singleton. In tests, we need to replace the value of a static field.

Reproduction Steps

using System.Reflection;

typeof(Singleton)
    .GetField("<Value>k__BackingField", BindingFlags.Static | BindingFlags.NonPublic)
    .SetValue(obj: null, value: 2); // System.FieldAccessException: Cannot set initonly static field '<Value>k__BackingField' after type 'Singleton' is initialized.
    
public static class Singleton
{
    public static int Value { get; } = 1;
}

Expected behavior

The value is set correctly without errors.

Actual behavior

System.FieldAccessException: Cannot set initonly static field 'k__BackingField' after type 'Singleton' is initialized.

Regression?

Yes. It worked fine in the .NET Framework

Known Workarounds

There are no known workarounds because, based on the discussion in the linked issues, changing static fields after type initialization can lead to broken behavior due to optimizations already applied.

Configuration

.NET8

Other information

We kindly ask you to take the community's requests as seriously as possible. This "optimization" broke many packages and solutions. It comes out at the moment when the enterprise migrates from the old .NET Framework to the new .NET.

I am not asking you to roll back these changes (although I do ask you to analyze whether they are really needed).

But please provide a CONVENIENT option to disable them at the solution / build configuration level.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 1, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jul 1, 2024
@colejohnson66
Copy link

colejohnson66 commented Jul 1, 2024

Modifying the backing field of init-only properties has never been defined behavior. Reflection always has the possibility to break things, and what you are doing was unsupported. So it broke when you upgraded.

The change to throw an exception was made such that, instead of silently failing in the future, it fails fast and load.

If you want a singleton that is settable, just add a set accessor. If you want to enforce some invariants, make it a private set and add a class-level method to set it. What you are doing now is broken.

@huoyaoyuan
Copy link
Member

  1. Overriding the default behavior of .NET and 3rd-party types built around the use of static APIs such as ThreaPool. For example, we use this to impose an end-to-end limit on how many Parallel.ForEachs can run at the same time.

This is unsupported case. Many internal code are implying the immutability of static readonly fields and behave depend on them.
The supported mechanism to tweak behavior of BCL apis is using AppContext switches.

  1. At any time, you can carry out a refactoring that will make the readonly field we need and this technique will stop working.

Private fields can be refactored away in any versions. There's strictly no guarantee that reflection on them can work across versions.

But please provide a CONVENIENT option to disable them at the solution / build configuration level.

It's hard to be convenient. The behavior has been employed by different components, including the JIT.

@teo-tsirpanis teo-tsirpanis added area-System.Reflection and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 1, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

@tannergooding
Copy link
Member

As was mentioned there are ways to still mutate the underlying field, such as via UnsafeAccessor, but such mutations are fundamentally unsafe and can lead to UB. If you need to mutate a field, don't mark it as readonly and provide an appropriate way to perform the mutation instead.

Mock dependencies in tests. For example, we use a third-party library that provides an interface for interacting with DNS via a singleton. In tests, we need to replace the value of a static field.

We have such similar things in our own code, we simply drop the const or readonly for DEBUG allowing it to be mutated for that scenario: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.SquMul.cs

The tests can then use a special helper that knows whether or not modifying the variable is supported:

BigIntTools.Utils.RunWithFakeThreshold(BigIntegerCalculator.SquareThreshold, 8, () =>
and
public static void RunWithFakeThreshold(in int field, int value, Action action)

Alternatively you can utilize features such as AppContext switches, environment variables, or other mechanisms to ensure the initial value ends up correct.

Overriding the default behavior of .NET and 3rd-party types built around the use of static APIs such as ThreaPool. For example, we use this to impose an end-to-end limit on how many Parallel.ForEachs can run at the same time. At any time, you can carry out a refactoring that will make the readonly field we need and this technique will stop working.

The same general considerations exist here, it is fairly trivial to have an AppContext switch, environment variable, or other input to seed a static readonly field allowing either user or per process customization. In some cases .NET provides its own context switches that allow you to customize the behavior (such as controlling GC configuration, limits in ASP.NET, etc).

@hez2010
Copy link
Contributor

hez2010 commented Jul 1, 2024

As was mentioned there are ways to still mutate the underlying field, such as via UnsafeAccessor, but such mutations are fundamentally unsafe and can lead to UB.

Mutating initonly static fields using UnsafeAccessor is broken with optimization, so I have deleted the comment suggesting UnsafeAccessor :)

@tannergooding
Copy link
Member

Right, that's why I made sure to call out that it's unsafe and can lead to UB.

readonly is considered an invariant and there are platforms, fields, and other scenarios that may require such data to go into readonly memory (such as for RVA statics or method local constants that go into otherwise "executable" memory). Further, because it is an invariant there are various systems and other components that take a dependency on this and assume the value will never be mutated once it's been initialized.

This isn't just an optimization, but rather a fundamental expectation of the system and that impacts many areas from security, to robustness, and also performance. If you have something that is fundamentally not readonly, then you shouldn't mark it as such.

@huoyaoyuan
Copy link
Member

If you have something that is fundamentally not readonly, then you shouldn't mark it as such.

The main ask was about components they don't own, including third party code and BCL types.

There is no really reliable way to modify static readonly fields, but some simple cases may just run well.

Using UnsafeAccessor can be an alternative way, but do remember the fundamental unreliability here.

@jkotas
Copy link
Member

jkotas commented Jul 1, 2024

As others commented, this behavior is by design. The immutability of readonly fields is assumed by multiple parts of the system.

Overriding the default behavior of .NET and 3rd-party types

If you need to modify behavior of .NET and 3rd-party components in ways that these components to do support natively, it is best to create your own build of those components with the required modifications, assuming that their license allows it.

@jkotas jkotas closed this as completed Jul 1, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Jul 1, 2024
@jkotas
Copy link
Member

jkotas commented Jul 1, 2024

Yes. It worked fine in the .NET Framework

It did not work reliably in .NET Framework. .NET Framework ignored the new value of readonly variable some of the time.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants