Skip to content

Commit

Permalink
Fix race in creating new unassigned global
Browse files Browse the repository at this point in the history
When a variable does not exist but we wish to acquire the
GlobalVariable object for it, this logic will attempt to create
a new GlobalVariable and add it to the map. Unfortunately it does
this without any locking or atomicity, potentially overwriting a
value written to the same name on a different thread.

When the GlobalVariable returned by this method is cached, as it
is in the JIT logic for global variable reads, it may never see
updates because those writes are going to the winner of the race.
This caused at least one set of specs, for Process.kill, to hang
when the global variable used for indicating a signal had been
damaged in this way.

Instead we computeIfAbsent to ensure only one GlobalVariable will
ever be inserted for this name.
  • Loading branch information
headius committed Nov 3, 2023
1 parent 7e111f3 commit 0a69e63
Showing 1 changed file with 2 additions and 14 deletions.
16 changes: 2 additions & 14 deletions core/src/main/java/org/jruby/internal/runtime/GlobalVariables.java
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,6 @@ public GlobalVariable getVariable(String name) {
assert name != null;
assert name.startsWith("$");

GlobalVariable variable = globalVariables.get(name);
if (variable != null) return variable;

return createIfNotDefined(name);
}

Expand All @@ -139,11 +136,7 @@ public IRubyObject clear(String name) {
}

public void setTraceVar(String name, RubyProc proc) {
assert name != null;
assert name.startsWith("$");

GlobalVariable variable = createIfNotDefined(name);
variable.addTrace(proc);
getVariable(name).addTrace(proc);
}

public boolean untraceVar(String name, IRubyObject command) {
Expand Down Expand Up @@ -172,12 +165,7 @@ public Set<String> getNames() {
}

private GlobalVariable createIfNotDefined(String name) {
GlobalVariable variable = globalVariables.get(name);
if (variable == null) {
variable = GlobalVariable.newUndefined(runtime, name);
globalVariables.put(name, variable);
}
return variable;
return globalVariables.computeIfAbsent(name, (n) -> GlobalVariable.newUndefined(runtime, n));
}

private IRubyObject defaultSeparator;
Expand Down

0 comments on commit 0a69e63

Please sign in to comment.