Skip to content

Commit 3724163

Browse files
authored
Fix Symbol.iterator JSReference cache (#453)
1 parent 437b69f commit 3724163

File tree

1 file changed

+36
-7
lines changed

1 file changed

+36
-7
lines changed

src/NodeApi/JSSymbol.cs

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,9 @@ namespace Microsoft.JavaScript.NodeApi;
1010
{
1111
private readonly JSValue _value;
1212

13-
//TODO: [vmoroz] This is a bug. we must never use static variables for JSReference or JSValue
14-
private static readonly Lazy<JSReference> s_iteratorSymbol =
15-
new(() => new JSReference(JSValue.Global["Symbol"]["iterator"]));
16-
private static readonly Lazy<JSReference> s_asyncIteratorSymbol =
17-
new(() => new JSReference(JSValue.Global["Symbol"]["asyncIterator"]));
13+
// Cached symbol references are thread-local because they must be initialized on each JS thread.
14+
[ThreadStatic] private static JSReference? s_iteratorSymbol;
15+
[ThreadStatic] private static JSReference? s_asyncIteratorSymbol;
1816

1917
/// <summary>
2018
/// Implicitly converts a <see cref="JSSymbol" /> to a <see cref="JSValue" />.
@@ -143,14 +141,45 @@ public string? Description
143141
}
144142
}
145143

144+
/// <summary>
145+
/// Gets or creates a symbol with the specified name in the global symbol registry.
146+
/// </summary>
146147
public static JSSymbol For(string name)
147148
{
148149
return new JSSymbol(JSValue.SymbolFor(name));
149150
}
150151

151-
public static JSSymbol Iterator => (JSSymbol)s_iteratorSymbol.Value.GetValue()!;
152+
/// <summary>
153+
/// Gets a well-known symbol by its name.
154+
/// </summary>
155+
public static JSSymbol Get(string name)
156+
{
157+
return (JSSymbol)JSValue.Global["Symbol"][name];
158+
}
159+
160+
private static JSSymbol Get(string name, ref JSReference? symbolReference)
161+
{
162+
if (symbolReference == null)
163+
{
164+
JSSymbol symbol = Get(name);
165+
symbolReference = new JSReference(symbol);
166+
return symbol;
167+
}
168+
else
169+
{
170+
return (JSSymbol)symbolReference.GetValue();
171+
}
172+
}
173+
174+
/// <summary>
175+
/// Gets the well-known symbol for the default iterator.
176+
/// </summary>
177+
public static JSSymbol Iterator => Get("iterator", ref s_iteratorSymbol);
152178

153-
public static JSSymbol AsyncIterator => (JSSymbol)s_asyncIteratorSymbol.Value.GetValue()!;
179+
/// <summary>
180+
/// Gets the well-known symbol for the async iterator.
181+
/// </summary>
182+
public static JSSymbol AsyncIterator => Get("asyncIterator", ref s_asyncIteratorSymbol);
154183

155184
// TODO: Add static properties for other well-known symbols.
156185

0 commit comments

Comments
 (0)