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

ResourceManager not threadsafe when case insensitive keys are used #80790

Open
tl-laurence opened this issue Jan 18, 2023 · 9 comments
Open

ResourceManager not threadsafe when case insensitive keys are used #80790

tl-laurence opened this issue Jan 18, 2023 · 9 comments
Labels
area-System.Resources needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Milestone

Comments

@tl-laurence
Copy link

tl-laurence commented Jan 18, 2023

Description

Using resource manager in a high load environment we see errors occurring and strings being read incorrectly. This only happens when the IgnoreCase property is true.

Error message: Corrupt .resources file. Invalid offset '503403335' into data section.

Stack:
at System.Resources.ResourceReader.AllocateStringForNameIndex(Int32 index, Int32& dataOffset) at System.Resources.RuntimeResourceSet.GetObject(String key, Boolean ignoreCase, Boolean isString) at System.Resources.RuntimeResourceSet.GetString(String key, Boolean ignoreCase) at System.Resources.ResourceManager.GetString(String name, CultureInfo culture)

Reproduction Steps

This test fails intermittently but more frequently after a clean rebuild

[TestFixture]
public class ResourceManagerTest
{
    [Test]
    public void GivenCaseInvariantResources_ThenNoErrors()
    {
        Parallel.For(0, 1000, (i) =>
         {
             var resourceMan = new System.Resources.ResourceManager("MyBaseName", typeof(MyAssembly).Assembly);
             resourceMan.IgnoreCase = true;

             Parallel.For(0, 1000, (i) =>
             {
                 var key = "key_that_exists_but_not_in_same_case_as_resources";
                 resourceMan.GetString(key);
             });
         });
    }
}

Expected behavior

The resource should be read without an error as the class is marked as thread safe.

Actual behavior

Error message: Corrupt .resources file. Invalid offset '503403335' into data section.

Stack:
at System.Resources.ResourceReader.AllocateStringForNameIndex(Int32 index, Int32& dataOffset) at System.Resources.RuntimeResourceSet.GetObject(String key, Boolean ignoreCase, Boolean isString) at System.Resources.RuntimeResourceSet.GetString(String key, Boolean ignoreCase) at System.Resources.ResourceManager.GetString(String name, CultureInfo culture)

After the error occurs once resource strings are read incorrectly without and further errors until the application is restarted

Regression?

No response

Known Workarounds

Don't use the IgnoreCase flag

Configuration

.NET 6.0
Errors occurs on:

  • Windows 10 Pro x64
  • Ubuntu 18.4 x64

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 18, 2023
@ghost
Copy link

ghost commented Jan 18, 2023

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

Issue Details

Description

Using resource manager in a high load environment we see errors occurring and strings being read incorrectly. This only happens when the IgnoreCase property is true.

Error message: Corrupt .resources file. Invalid offset '503403335' into data section.

Stack:
at System.Resources.ResourceReader.AllocateStringForNameIndex(Int32 index, Int32& dataOffset) at System.Resources.RuntimeResourceSet.GetObject(String key, Boolean ignoreCase, Boolean isString) at System.Resources.RuntimeResourceSet.GetString(String key, Boolean ignoreCase) at System.Resources.ResourceManager.GetString(String name, CultureInfo culture)

Reproduction Steps

This test fails intermittently but more frequently after a clean rebuild

[TestFixture]
    public class ResourceManagerTest
    {
        [Test]
        public void GivenCaseInvariantResources_ThenNoErrors()
        {
            Parallel.For(0, 1000, (i) =>
             {
                 var resourceMan = new System.Resources.ResourceManager("MyBaseName", typeof(MyAssembly).Assembly);
                 resourceMan.IgnoreCase = true;

                 Parallel.For(0, 1000, (i) =>
                 {
                     var key = "key_that_exists_but_not_in_same_case_as_resources";
                     resourceMan.GetString(key);
                 });
             });
        }
    }`

### Expected behavior

The resource should be read without an error as the class is marked as thread safe.

### Actual behavior

Error message: `Corrupt .resources file. Invalid offset '503403335' into data section.`

Stack:
`
  at System.Resources.ResourceReader.AllocateStringForNameIndex(Int32 index, Int32& dataOffset)
   at System.Resources.RuntimeResourceSet.GetObject(String key, Boolean ignoreCase, Boolean isString)
   at System.Resources.RuntimeResourceSet.GetString(String key, Boolean ignoreCase)
   at System.Resources.ResourceManager.GetString(String name, CultureInfo culture)
`

After the error occurs once **resource strings are read incorrectly without and further errors** until the application is restarted

### Regression?

_No response_

### Known Workarounds

Don't use the IgnoreCase flag

### Configuration

.NET 6.0
Errors occurs on:
 - Windows 10 Pro x64
 - Ubuntu 18.4 x64

### Other information

_No response_

<table>
  <tr>
    <th align="left">Author:</th>
    <td>tl-laurence</td>
  </tr>
  <tr>
    <th align="left">Assignees:</th>
    <td>-</td>
  </tr>
  <tr>
    <th align="left">Labels:</th>
    <td>

`area-System.Resources`, `untriaged`

</td>
  </tr>
  <tr>
    <th align="left">Milestone:</th>
    <td>-</td>
  </tr>
</table>
</details>

@steveharter
Copy link
Member

steveharter commented Jan 18, 2023

Appears to be a bug however we need to verify locally. That method is doc'd as being "thread safe"

Thread Safety
The GetString(String) method is thread safe.

@steveharter
Copy link
Member

@tl-laurence is the resource file special in any way (lots of items?) or is it changed at runtime?

Basically wondering how to best repro this.

@buyaa-n buyaa-n added this to the Future milestone Jan 18, 2023
@steveharter steveharter removed the untriaged New issue has not been triaged by the area owner label Jan 18, 2023
@buyaa-n buyaa-n added the needs-author-action An issue or pull request that requires more info or actions from the author. label Jan 18, 2023
@tarekgh
Copy link
Member

tarekgh commented Jan 18, 2023

Is this different than #74052?

@ghost
Copy link

ghost commented Jan 18, 2023

This issue has been marked needs-author-action and may be missing some important information.

@buyaa-n
Copy link
Contributor

buyaa-n commented Jan 18, 2023

Looks quite similar thanks @tarekgh, @tl-laurence could you try with recent .NET 8 bits where #74052 fixed?

@tl-laurence
Copy link
Author

tl-laurence commented Jan 19, 2023

Yes it looks like it could be the same issue as #74052. I will have a go at testing with the suggested version.

@steveharter there is nothing special about the resource files. We have multiple languages configured but can reproduce with a single culture.

It is the IgnoreCase flag and the use of a key that is not in the correct case that causes it from what I have found.

We removed the ignorecase and looked up the keys in a concurrent dictionary instead as a workaround which has stopped the errors.

@ghost ghost added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Jan 19, 2023
@danmoseley
Copy link
Member

@tl-laurence did you verify it's fixed?

@tl-laurence
Copy link
Author

Yes this is fixed in .Net 8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Resources needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Projects
None yet
Development

No branches or pull requests

5 participants