Skip to content

Commit

Permalink
Revert "Use Dictionary for underlying cache of ResourceSet (#44104)" (#…
Browse files Browse the repository at this point in the history 
…44482)

This reverts commit a683543.
  • Loading branch information
@jkotas
jkotas authored Nov 10, 2020
1 parent a79df14 commit a5c0b09
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 107 deletions.
107 changes: 64 additions & 43 deletions src/libraries/System.Private.CoreLib/src/System/Resources/ResourceSet.cs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

/*============================================================
**
**
**
**
**
** Purpose: Culture-specific collection of resources.
**
**
===========================================================*/

using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
Expand All @@ -19,15 +29,15 @@ namespace System.Resources
public class ResourceSet : IDisposable, IEnumerable
{
protected IResourceReader Reader = null!;
internal Hashtable? Table;

private Dictionary<object, object?>? _table;
private Dictionary<string, object?>? _caseInsensitiveTable; // For case-insensitive lookups.
private Hashtable? _caseInsensitiveTable; // For case-insensitive lookups.

protected ResourceSet()
{
// To not inconvenience people subclassing us, we should allocate a new
// hashtable here just so that Table is set to something.
_table = new Dictionary<object, object?>();
Table = new Hashtable();
}

// For RuntimeResourceSet, ignore the Table parameter - it's a wasted
Expand Down Expand Up @@ -86,7 +96,7 @@ protected virtual void Dispose(bool disposing)
}
Reader = null!;
_caseInsensitiveTable = null;
_table = null;
Table = null;
}

public void Dispose()
Expand Down Expand Up @@ -124,46 +134,59 @@ IEnumerator IEnumerable.GetEnumerator()

private IDictionaryEnumerator GetEnumeratorHelper()
{
// Avoid a race with Dispose
return _table?.GetEnumerator() ?? throw new ObjectDisposedException(null, SR.ObjectDisposed_ResourceSet);
Hashtable? copyOfTable = Table; // Avoid a race with Dispose
if (copyOfTable == null)
throw new ObjectDisposedException(null, SR.ObjectDisposed_ResourceSet);
return copyOfTable.GetEnumerator();
}

// Look up a string value for a resource given its name.
//
public virtual string? GetString(string name)
{
object? obj = GetObjectInternal(name);
if (obj is string s)
return s;

if (obj is null)
return null;

throw new InvalidOperationException(SR.Format(SR.InvalidOperation_ResourceNotString_Name, name));
try
{
return (string?)obj;
}
catch (InvalidCastException)
{
throw new InvalidOperationException(SR.Format(SR.InvalidOperation_ResourceNotString_Name, name));
}
}

public virtual string? GetString(string name, bool ignoreCase)
{
// Case-sensitive lookup
object? obj = GetObjectInternal(name);
if (obj is string s)
return s;
object? obj;
string? s;

if (obj is not null)
// Case-sensitive lookup
obj = GetObjectInternal(name);
try
{
s = (string?)obj;
}
catch (InvalidCastException)
{
throw new InvalidOperationException(SR.Format(SR.InvalidOperation_ResourceNotString_Name, name));
}

if (!ignoreCase)
return null;
// case-sensitive lookup succeeded
if (s != null || !ignoreCase)
{
return s;
}

// Try doing a case-insensitive lookup
obj = GetCaseInsensitiveObjectInternal(name);
if (obj is string si)
return si;

if (obj is null)
return null;

throw new InvalidOperationException(SR.Format(SR.InvalidOperation_ResourceNotString_Name, name));
try
{
return (string?)obj;
}
catch (InvalidCastException)
{
throw new InvalidOperationException(SR.Format(SR.InvalidOperation_ResourceNotString_Name, name));
}
}

// Look up an object value for a resource given its name.
Expand All @@ -185,12 +208,13 @@ private IDictionaryEnumerator GetEnumeratorHelper()

protected virtual void ReadResources()
{
Debug.Assert(_table != null);
Debug.Assert(Table != null);
Debug.Assert(Reader != null);
IDictionaryEnumerator en = Reader.GetEnumerator();
while (en.MoveNext())
{
_table.Add(en.Key, en.Value);
object? value = en.Value;
Table.Add(en.Key, value);
}
// While technically possible to close the Reader here, don't close it
// to help with some WinRes lifetime issues.
Expand All @@ -201,38 +225,35 @@ protected virtual void ReadResources()
if (name == null)
throw new ArgumentNullException(nameof(name));

Dictionary<object, object?>? copyOfTable = _table; // Avoid a race with Dispose
Hashtable? copyOfTable = Table; // Avoid a race with Dispose

if (copyOfTable == null)
throw new ObjectDisposedException(null, SR.ObjectDisposed_ResourceSet);

copyOfTable.TryGetValue(name, out object? value);
return value;
return copyOfTable[name];
}

private object? GetCaseInsensitiveObjectInternal(string name)
{
Dictionary<object, object?>? copyOfTable = _table; // Avoid a race with Dispose
Hashtable? copyOfTable = Table; // Avoid a race with Dispose

if (copyOfTable == null)
throw new ObjectDisposedException(null, SR.ObjectDisposed_ResourceSet);

Dictionary<string, object?>? caseTable = _caseInsensitiveTable; // Avoid a race condition with Close
Hashtable? caseTable = _caseInsensitiveTable; // Avoid a race condition with Close
if (caseTable == null)
{
caseTable = new Dictionary<string, object?>(copyOfTable.Count, StringComparer.OrdinalIgnoreCase);
foreach (var item in copyOfTable)
{
if (item.Key is not string s)
continue;
caseTable = new Hashtable(StringComparer.OrdinalIgnoreCase);

caseTable.Add(s, item.Value);
IDictionaryEnumerator en = copyOfTable.GetEnumerator();
while (en.MoveNext())
{
caseTable.Add(en.Key, en.Value);
}
_caseInsensitiveTable = caseTable;
}

caseTable.TryGetValue(name, out object? value);
return value;
return caseTable[name];
}
}
}
64 changes: 0 additions & 64 deletions src/libraries/System.Resources.ResourceManager/tests/ResourceSetTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.IO;
using System.Reflection;
using System.Globalization;
using System.Collections;
using System.Collections.Generic;

namespace System.Resources.Tests
Expand Down Expand Up @@ -102,69 +101,6 @@ public void GetString()
}
}

public class ResourceSetTests_IResourceReader
{
class SimpleResourceReader : IResourceReader
{
Hashtable data;
public SimpleResourceReader()
{
data = new Hashtable();
data.Add(1, "invalid");
data.Add("String", "message");
data.Add("Int32", 5);
}

IEnumerator IEnumerable.GetEnumerator()
{
throw new NotSupportedException();
}

public IDictionaryEnumerator GetEnumerator()
{
return data.GetEnumerator();
}

public void Close()
{
}

public void Dispose()
{
}
}

[Fact]
public void Empty_Ctor()
{
Assert.Throws<ArgumentNullException>(() => new ResourceSet(null as IResourceReader));
}

[Fact]
public void GetObject()
{
var rs = new ResourceSet(new SimpleResourceReader());

Assert.Null(rs.GetObject("DoesNotExist"));
Assert.Null(rs.GetObject("1"));

Assert.Equal(5, rs.GetObject("Int32"));
Assert.Equal(5, rs.GetObject("int32", true));
}

[Fact]
public void GetString()
{
var rs = new ResourceSet(new SimpleResourceReader());

Assert.Null(rs.GetString("DoesNotExist"));
Assert.Null(rs.GetString("1"));

Assert.Equal("message", rs.GetString("String"));
Assert.Equal("message", rs.GetString("string", true));
}
}

public class ResourceSetTests_StreamCtor : ResourceSetTests
{
public override ResourceSet GetSet(string base64Data)
Expand Down

0 comments on commit a5c0b09

Please sign in to comment.