Skip to content

Commit 241f9db

Browse files
author
Zach Olstein
committed
Fix race condition when contexts share an engine
This patch includes fixes for a category of race conditions when concurrently running code in two or more contexts that share an engine. The PythonNAryClinicBuiltinNodes can have multiple threads call castWithNode concurrently on the same object. After one thread has assigned a particular castNode, another can overwrite the array with an empty one, causing an NPE when re-reading it from the array. Instead, the methods can assign directly a local variable and then save the values in the array. With this change, a thread doesn't risk reading a null after having computed the value.
1 parent 4bca05e commit 241f9db

File tree

3 files changed

+18
-9
lines changed

3 files changed

+18
-9
lines changed

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/function/builtins/PythonBinaryClinicBuiltinNode.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,18 @@ private Object cast(ArgumentClinicProvider clinic, VirtualFrame frame, int argIn
6868
}
6969

7070
protected Object castWithNode(ArgumentClinicProvider clinic, VirtualFrame frame, int argIndex, Object value) {
71+
ArgumentCastNode castNode;
7172
if (castNodes == null) {
7273
CompilerDirectives.transferToInterpreterAndInvalidate();
7374
castNodes = new ArgumentCastNode[2];
7475
}
75-
if (castNodes[argIndex] == null) {
76+
castNode = castNodes[argIndex];
77+
if (castNode == null) {
7678
CompilerDirectives.transferToInterpreterAndInvalidate();
77-
castNodes[argIndex] = insert(clinic.createCastNode(argIndex, this));
79+
castNode = insert(clinic.createCastNode(argIndex, this));
80+
castNodes[argIndex] = castNode;
7881
}
79-
return castNodes[argIndex].execute(frame, value);
82+
return castNode.execute(frame, value);
8083
}
8184

8285
@Override

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/function/builtins/PythonQuaternaryClinicBuiltinNode.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,18 @@ private Object cast(ArgumentClinicProvider clinic, VirtualFrame frame, int argIn
6666
}
6767

6868
protected Object castWithNode(ArgumentClinicProvider clinic, VirtualFrame frame, int argIndex, Object value) {
69+
ArgumentCastNode castNode;
6970
if (castNodes == null) {
7071
CompilerDirectives.transferToInterpreterAndInvalidate();
7172
castNodes = new ArgumentCastNode[4];
7273
}
73-
if (castNodes[argIndex] == null) {
74+
castNode = castNodes[argIndex];
75+
if (castNode == null) {
7476
CompilerDirectives.transferToInterpreterAndInvalidate();
75-
castNodes[argIndex] = insert(clinic.createCastNode(argIndex, this));
77+
castNode = insert(clinic.createCastNode(argIndex, this));
78+
castNodes[argIndex] = castNode;
7679
}
77-
return castNodes[argIndex].execute(frame, value);
80+
return castNode.execute(frame, value);
7881
}
7982

8083
@Override

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/function/builtins/PythonTernaryClinicBuiltinNode.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,18 @@ private Object cast(ArgumentClinicProvider clinic, VirtualFrame frame, int argIn
6666
}
6767

6868
protected Object castWithNode(ArgumentClinicProvider clinic, VirtualFrame frame, int argIndex, Object value) {
69+
ArgumentCastNode castNode;
6970
if (castNodes == null) {
7071
CompilerDirectives.transferToInterpreterAndInvalidate();
7172
castNodes = new ArgumentCastNode[3];
7273
}
73-
if (castNodes[argIndex] == null) {
74+
castNode = castNodes[argIndex];
75+
if (castNode == null) {
7476
CompilerDirectives.transferToInterpreterAndInvalidate();
75-
castNodes[argIndex] = insert(clinic.createCastNode(argIndex, this));
77+
castNode = insert(clinic.createCastNode(argIndex, this));
78+
castNodes[argIndex] = castNode;
7679
}
77-
return castNodes[argIndex].execute(frame, value);
80+
return castNode.execute(frame, value);
7881
}
7982

8083
@Override

0 commit comments

Comments
 (0)