Skip to content

Commit aa6623d

Browse files
eregoneregon
committed
[GR-15179] Correctly use the cached Shape to create the shared Shape in ShareObjectNode.
PullRequest: truffleruby/799
2 parents 85bbbf0 + 6781991 commit aa6623d

File tree

6 files changed

+134
-26
lines changed

6 files changed

+134
-26
lines changed

‎src/main/java/org/truffleruby/language/objects/shared/ShareObjectNode.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,14 @@ public ShareObjectNode(int depth) {
4646

4747
@Specialization(
4848
guards = "object.getShape() == cachedShape",
49-
assumptions = "cachedShape.getValidAssumption()",
49+
assumptions = { "cachedShape.getValidAssumption()", "sharedShape.getValidAssumption()" },
5050
limit = "CACHE_LIMIT")
5151
@ExplodeLoop
5252
protected void shareCached(DynamicObject object,
5353
@Cached("ensureSharedClasses(getContext(), object.getShape())") Shape cachedShape,
5454
@Cached("createShareInternalFieldsNode()") ShareInternalFieldsNode shareInternalFieldsNode,
5555
@Cached("createReadAndShareFieldNodes(getObjectProperties(cachedShape))") ReadAndShareFieldNode[] readAndShareFieldNodes,
56-
@Cached("createSharedShape(object)") Shape sharedShape) {
57-
assert !SharedObjects.isShared(getContext(), cachedShape);
58-
56+
@Cached("createSharedShape(cachedShape)") Shape sharedShape) {
5957
// Mark the object as shared first to avoid recursion
6058
object.setShapeAndGrow(cachedShape, sharedShape);
6159

@@ -110,10 +108,12 @@ protected ReadAndShareFieldNode[] createReadAndShareFieldNodes(List<Property> pr
110108
return nodes;
111109
}
112110

113-
protected static Shape createSharedShape(DynamicObject object) {
114-
object.updateShape();
115-
final Shape oldShape = object.getShape();
116-
return oldShape.makeSharedShape();
111+
protected Shape createSharedShape(Shape cachedShape) {
112+
if (SharedObjects.isShared(getContext(), cachedShape)) {
113+
throw new UnsupportedOperationException("Thread-safety bug: the object is already shared. This means another thread marked the object as shared concurrently.");
114+
} else {
115+
return cachedShape.makeSharedShape();
116+
}
117117
}
118118

119119
}

‎src/main/java/org/truffleruby/language/objects/shared/SharedObjects.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public static boolean isShared(RubyContext context, DynamicObject object) {
107107
}
108108

109109
public static boolean isShared(RubyContext context, Shape shape) {
110-
return context.getOptions().SHARED_OBJECTS_ENABLED && (context.getOptions().SHARED_OBJECTS_SHARE_ALL || shape.isShared());
110+
return context.getOptions().SHARED_OBJECTS_ENABLED && shape.isShared();
111111
}
112112

113113
public static void writeBarrier(RubyContext context, Object value) {

‎src/main/java/org/truffleruby/options/Options.java

Lines changed: 123 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,130 +23,252 @@
2323
@Generated("tool/generate-options.rb")
2424
public class Options {
2525

26+
/** --load-paths=new String[]{} */
2627
public final String[] LOAD_PATHS;
28+
/** --required-libraries=new String[]{} */
2729
public final String[] REQUIRED_LIBRARIES;
30+
/** --working-directory="" */
2831
public final String WORKING_DIRECTORY;
32+
/** --debug=false */
2933
public final boolean DEBUG;
34+
/** --verbose=Verbosity.FALSE */
3035
public final Verbosity VERBOSITY;
36+
/** --source-encoding="" */
3137
public final String SOURCE_ENCODING;
38+
/** --internal-encoding="" */
3239
public final String INTERNAL_ENCODING;
40+
/** --external-encoding="" */
3341
public final String EXTERNAL_ENCODING;
42+
/** --home="" */
3443
public final String HOME;
44+
/** --no-home-provided=false */
3545
public final boolean NO_HOME_PROVIDED;
46+
/** --launcher="" */
3647
public final String LAUNCHER;
48+
/** --core-load-path="resource:/truffleruby" */
3749
public final String CORE_LOAD_PATH;
50+
/** --frozen-string-literals=false */
3851
public final boolean FROZEN_STRING_LITERALS;
52+
/** --rubygems=true */
3953
public final boolean RUBYGEMS;
54+
/** --lazy-default=true */
4055
public final boolean DEFAULT_LAZY;
56+
/** --rubygems-lazy=DEFAULT_LAZY */
4157
public final boolean LAZY_RUBYGEMS;
58+
/** --patching=true */
4259
public final boolean PATCHING;
60+
/** --did-you-mean=true */
4361
public final boolean DID_YOU_MEAN;
62+
/** --hashing-deterministic=false */
4463
public final boolean HASHING_DETERMINISTIC;
64+
/** --embedded=true */
4565
public final boolean EMBEDDED;
66+
/** --platform-native=env.isNativeAccessAllowed() && true */
4667
public final boolean NATIVE_PLATFORM;
68+
/** --platform-native-interrupt=NATIVE_PLATFORM */
4769
public final boolean NATIVE_INTERRUPT;
70+
/** --platform-handle-interrupt=!EMBEDDED */
4871
public final boolean HANDLE_INTERRUPT;
72+
/** --single-threaded=!env.isCreateThreadAllowed() || EMBEDDED */
4973
public final boolean SINGLE_THREADED;
74+
/** --polyglot-stdio=EMBEDDED || !NATIVE_PLATFORM */
5075
public final boolean POLYGLOT_STDIO;
76+
/** --interop-host=env.isHostLookupAllowed() && true */
5177
public final boolean HOST_INTEROP;
78+
/** --trace-calls=true */
5279
public final boolean TRACE_CALLS;
80+
/** --coverage-global=false */
5381
public final boolean COVERAGE_GLOBAL;
82+
/** --core-as-internal=true */
5483
public final boolean CORE_AS_INTERNAL;
84+
/** --stdlib-as-internal=true */
5585
public final boolean STDLIB_AS_INTERNAL;
86+
/** --lazy-translation-user=false */
5687
public final boolean LAZY_TRANSLATION_USER;
88+
/** --exceptions-store-java=false */
5789
public final boolean EXCEPTIONS_STORE_JAVA;
90+
/** --exceptions-print-java=false */
5891
public final boolean EXCEPTIONS_PRINT_JAVA;
92+
/** --exceptions-print-uncaught-java=false */
5993
public final boolean EXCEPTIONS_PRINT_UNCAUGHT_JAVA;
94+
/** --exceptions-print-ruby-for-java=false */
6095
public final boolean EXCEPTIONS_PRINT_RUBY_FOR_JAVA;
96+
/** --exceptions-translate-assert=true */
6197
public final boolean EXCEPTIONS_TRANSLATE_ASSERT;
98+
/** --exceptions-warn-stackoverflow=true */
6299
public final boolean EXCEPTIONS_WARN_STACKOVERFLOW;
100+
/** --exceptions-warn-out-of-memory=true */
63101
public final boolean EXCEPTIONS_WARN_OUT_OF_MEMORY;
102+
/** --backtraces-hide-core-files=true */
64103
public final boolean BACKTRACES_HIDE_CORE_FILES;
104+
/** --backtraces-interleave-java=false */
65105
public final boolean BACKTRACES_INTERLEAVE_JAVA;
106+
/** --backtraces-limit=9999 */
66107
public final int BACKTRACES_LIMIT;
108+
/** --backtraces-omit-unused=true */
67109
public final boolean BACKTRACES_OMIT_UNUSED;
110+
/** --backtraces-on-interrupt=false */
68111
public final boolean BACKTRACE_ON_INTERRUPT;
112+
/** --backtraces-sigalrm=!EMBEDDED */
69113
public final boolean BACKTRACE_ON_SIGALRM;
114+
/** --backtraces-raise=false */
70115
public final boolean BACKTRACE_ON_RAISE;
116+
/** --cexts=true */
71117
public final boolean CEXTS;
118+
/** --cexts-lock=true */
72119
public final boolean CEXT_LOCK;
120+
/** --cexts-remap=new String[]{} */
73121
public final String[] CEXTS_LIBRARY_REMAP;
122+
/** --options-log=false */
74123
public final boolean OPTIONS_LOG;
124+
/** --log-load=false */
75125
public final boolean LOG_LOAD;
126+
/** --log-autoload=false */
76127
public final boolean LOG_AUTOLOAD;
128+
/** --log-feature-location=false */
77129
public final boolean LOG_FEATURE_LOCATION;
130+
/** --cexts-log-load=false */
78131
public final boolean CEXTS_LOG_LOAD;
132+
/** --cexts-log-warnings=false */
79133
public final boolean CEXTS_LOG_WARNINGS;
134+
/** --argv-globals=false */
80135
public final boolean ARGV_GLOBALS;
136+
/** --ignore-lines-before-ruby-shebang=false */
81137
public final boolean IGNORE_LINES_BEFORE_RUBY_SHEBANG;
138+
/** --syntax-check=false */
82139
public final boolean SYNTAX_CHECK;
140+
/** --argv-global-values=new String[]{} */
83141
public final String[] ARGV_GLOBAL_VALUES;
142+
/** --argv-global-flags=new String[]{} */
84143
public final String[] ARGV_GLOBAL_FLAGS;
144+
/** --building-core-cexts=false */
85145
public final boolean BUILDING_CORE_CEXTS;
146+
/** --lazy-translation-log=false */
86147
public final boolean LAZY_TRANSLATION_LOG;
148+
/** --constant-dynamic-lookup-log=false */
87149
public final boolean LOG_DYNAMIC_CONSTANT_LOOKUP;
150+
/** --rope-print-intern-stats=false */
88151
public final boolean ROPE_PRINT_INTERN_STATS;
152+
/** --preinit=true */
89153
public final boolean PREINITIALIZATION;
154+
/** --lazy-builtins=DEFAULT_LAZY */
90155
public final boolean LAZY_BUILTINS;
156+
/** --lazy-core-method-nodes=DEFAULT_LAZY */
91157
public final boolean LAZY_CORE_METHOD_NODES;
158+
/** --lazy-translation-core=DEFAULT_LAZY */
92159
public final boolean LAZY_TRANSLATION_CORE;
160+
/** --basic-ops-inline=true */
93161
public final boolean BASICOPS_INLINE;
162+
/** --rope-lazy-substrings=true */
94163
public final boolean ROPE_LAZY_SUBSTRINGS;
164+
/** --default-cache=8 */
95165
public final int DEFAULT_CACHE;
166+
/** --method-lookup-cache=DEFAULT_CACHE */
96167
public final int METHOD_LOOKUP_CACHE;
168+
/** --dispatch-cache=DEFAULT_CACHE */
97169
public final int DISPATCH_CACHE;
170+
/** --yield-cache=DEFAULT_CACHE */
98171
public final int YIELD_CACHE;
172+
/** --to-proc-cache=DEFAULT_CACHE */
99173
public final int METHOD_TO_PROC_CACHE;
174+
/** --is-a-cache=DEFAULT_CACHE */
100175
public final int IS_A_CACHE;
176+
/** --bind-cache=DEFAULT_CACHE */
101177
public final int BIND_CACHE;
178+
/** --constant-cache=DEFAULT_CACHE */
102179
public final int CONSTANT_CACHE;
180+
/** --instance-variable-cache=DEFAULT_CACHE */
103181
public final int INSTANCE_VARIABLE_CACHE;
182+
/** --binding-local-variable-cache=DEFAULT_CACHE */
104183
public final int BINDING_LOCAL_VARIABLE_CACHE;
184+
/** --symbol-to-proc-cache=DEFAULT_CACHE */
105185
public final int SYMBOL_TO_PROC_CACHE;
186+
/** --allocate-class-cache=DEFAULT_CACHE */
106187
public final int ALLOCATE_CLASS_CACHE;
188+
/** --pack-cache=DEFAULT_CACHE */
107189
public final int PACK_CACHE;
190+
/** --unpack-cache=DEFAULT_CACHE */
108191
public final int UNPACK_CACHE;
192+
/** --eval-cache=DEFAULT_CACHE */
109193
public final int EVAL_CACHE;
194+
/** --class-cache=DEFAULT_CACHE */
110195
public final int CLASS_CACHE;
196+
/** --encoding-compatible-query-cache=DEFAULT_CACHE */
111197
public final int ENCODING_COMPATIBLE_QUERY_CACHE;
198+
/** --encoding-loaded-classes-cache=DEFAULT_CACHE */
112199
public final int ENCODING_LOADED_CLASSES_CACHE;
200+
/** --thread-cache=DEFAULT_CACHE */
113201
public final int THREAD_CACHE;
202+
/** --rope-class-cache=8 */
114203
public final int ROPE_CLASS_CACHE;
204+
/** --interop-convert-cache=DEFAULT_CACHE */
115205
public final int INTEROP_CONVERT_CACHE;
206+
/** --interop-execute-cache=DEFAULT_CACHE */
116207
public final int INTEROP_EXECUTE_CACHE;
208+
/** --interop-invoke-cache=DEFAULT_CACHE */
117209
public final int INTEROP_INVOKE_CACHE;
210+
/** --interop-new-cache=DEFAULT_CACHE */
118211
public final int INTEROP_NEW_CACHE;
212+
/** --time-format-cache=DEFAULT_CACHE */
119213
public final int TIME_FORMAT_CACHE;
214+
/** --integer-pow-cache=DEFAULT_CACHE */
120215
public final int POW_CACHE;
216+
/** --array-dup-cache=3 */
121217
public final int ARRAY_DUP_CACHE;
218+
/** --frame-variable-access-cache=5 */
122219
public final int FRAME_VARIABLE_ACCESS_CACHE;
220+
/** --array-uninitialized-size=16 */
123221
public final int ARRAY_UNINITIALIZED_SIZE;
222+
/** --array-small=3 */
124223
public final int ARRAY_SMALL;
224+
/** --hash-packed-array-max=3 */
125225
public final int HASH_PACKED_ARRAY_MAX;
226+
/** --pack-unroll=4 */
126227
public final int PACK_UNROLL_LIMIT;
228+
/** --pack-recover=32 */
127229
public final int PACK_RECOVER_LOOP_MIN;
230+
/** --cexts-marking-cache=100 */
128231
public final int CEXTS_MARKING_CACHE;
232+
/** --rope-depth-threshold=128 */
129233
public final int ROPE_DEPTH_THRESHOLD;
234+
/** --global-variable-max-invalidations=1 */
130235
public final int GLOBAL_VARIABLE_MAX_INVALIDATIONS;
236+
/** --clone-default=true */
131237
public final boolean CLONE_DEFAULT;
238+
/** --inline-default=true */
132239
public final boolean INLINE_DEFAULT;
240+
/** --core-always-clone=CLONE_DEFAULT */
133241
public final boolean CORE_ALWAYS_CLONE;
242+
/** --primitive-callers-always-clone=CLONE_DEFAULT */
134243
public final boolean PRIMITIVE_CALLERS_ALWAYS_CLONE;
244+
/** --always-split-honor=CLONE_DEFAULT */
135245
public final boolean ALWAYS_SPLIT_HONOR;
246+
/** --inline-needs-caller-frame=INLINE_DEFAULT */
136247
public final boolean INLINE_NEEDS_CALLER_FRAME;
248+
/** --yield-always-clone=CLONE_DEFAULT */
137249
public final boolean YIELD_ALWAYS_CLONE;
250+
/** --yield-always-inline=INLINE_DEFAULT */
138251
public final boolean YIELD_ALWAYS_INLINE;
252+
/** --method-missing-always-clone=CLONE_DEFAULT */
139253
public final boolean METHODMISSING_ALWAYS_CLONE;
254+
/** --method-missing-always-inline=INLINE_DEFAULT */
140255
public final boolean METHODMISSING_ALWAYS_INLINE;
256+
/** --call-with-block-always-clone=CLONE_DEFAULT */
141257
public final boolean CALL_WITH_BLOCK_ALWAYS_CLONE;
258+
/** --regexp-instrument-creation=false */
142259
public final boolean REGEXP_INSTRUMENT_CREATION;
260+
/** --regexp-instrument-match=false */
143261
public final boolean REGEXP_INSTRUMENT_MATCH;
262+
/** --metrics-time-parsing-file=false */
144263
public final boolean METRICS_TIME_PARSING_FILE;
264+
/** --metrics-time-require=false */
145265
public final boolean METRICS_TIME_REQUIRE;
266+
/** --shared-objects=true */
146267
public final boolean SHARED_OBJECTS_ENABLED;
268+
/** --shared-objects-debug=false */
147269
public final boolean SHARED_OBJECTS_DEBUG;
270+
/** --shared-objects-force=false */
148271
public final boolean SHARED_OBJECTS_FORCE;
149-
public final boolean SHARED_OBJECTS_SHARE_ALL;
150272

151273
public Options(Env env, OptionValues options) {
152274
LOAD_PATHS = options.get(OptionsCatalog.LOAD_PATHS_KEY);
@@ -272,7 +394,6 @@ public Options(Env env, OptionValues options) {
272394
SHARED_OBJECTS_ENABLED = options.get(OptionsCatalog.SHARED_OBJECTS_ENABLED_KEY);
273395
SHARED_OBJECTS_DEBUG = options.get(OptionsCatalog.SHARED_OBJECTS_DEBUG_KEY);
274396
SHARED_OBJECTS_FORCE = options.get(OptionsCatalog.SHARED_OBJECTS_FORCE_KEY);
275-
SHARED_OBJECTS_SHARE_ALL = options.get(OptionsCatalog.SHARED_OBJECTS_SHARE_ALL_KEY);
276397
}
277398

278399
public Object fromDescriptor(OptionDescriptor descriptor) {
@@ -523,8 +644,6 @@ public Object fromDescriptor(OptionDescriptor descriptor) {
523644
return SHARED_OBJECTS_DEBUG;
524645
case "ruby.shared-objects-force":
525646
return SHARED_OBJECTS_FORCE;
526-
case "ruby.shared-objects-share-all":
527-
return SHARED_OBJECTS_SHARE_ALL;
528647
default:
529648
return null;
530649
}

‎src/options.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,4 +170,3 @@ INTERNAL: # Options for debugging the TruffleRuby implementation
170170
SHARED_OBJECTS_ENABLED: [shared-objects, boolean, true, Enable thread-safe objects]
171171
SHARED_OBJECTS_DEBUG: [shared-objects-debug, boolean, false, Print information about shared objects]
172172
SHARED_OBJECTS_FORCE: [shared-objects-force, boolean, false, Force sharing of objects roots at startup]
173-
SHARED_OBJECTS_SHARE_ALL: [shared-objects-share-all, boolean, false, Consider all objects as shared]

‎src/shared/java/org/truffleruby/shared/options/OptionsCatalog.java

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,6 @@ public class OptionsCatalog {
144144
public static final OptionKey<Boolean> SHARED_OBJECTS_ENABLED_KEY = new OptionKey<>(true);
145145
public static final OptionKey<Boolean> SHARED_OBJECTS_DEBUG_KEY = new OptionKey<>(false);
146146
public static final OptionKey<Boolean> SHARED_OBJECTS_FORCE_KEY = new OptionKey<>(false);
147-
public static final OptionKey<Boolean> SHARED_OBJECTS_SHARE_ALL_KEY = new OptionKey<>(false);
148147

149148
public static final OptionDescriptor LOAD_PATHS = OptionDescriptor
150149
.newBuilder(LOAD_PATHS_KEY, "ruby.load-paths")
@@ -1007,13 +1006,6 @@ public class OptionsCatalog {
10071006
.stability(OptionStability.EXPERIMENTAL)
10081007
.build();
10091008

1010-
public static final OptionDescriptor SHARED_OBJECTS_SHARE_ALL = OptionDescriptor
1011-
.newBuilder(SHARED_OBJECTS_SHARE_ALL_KEY, "ruby.shared-objects-share-all")
1012-
.help("Consider all objects as shared")
1013-
.category(OptionCategory.INTERNAL)
1014-
.stability(OptionStability.EXPERIMENTAL)
1015-
.build();
1016-
10171009
public static OptionDescriptor fromName(String name) {
10181010
switch (name) {
10191011
case "ruby.load-paths":
@@ -1262,8 +1254,6 @@ public static OptionDescriptor fromName(String name) {
12621254
return SHARED_OBJECTS_DEBUG;
12631255
case "ruby.shared-objects-force":
12641256
return SHARED_OBJECTS_FORCE;
1265-
case "ruby.shared-objects-share-all":
1266-
return SHARED_OBJECTS_SHARE_ALL;
12671257
default:
12681258
return null;
12691259
}
@@ -1394,7 +1384,6 @@ public static OptionDescriptor[] allDescriptors() {
13941384
SHARED_OBJECTS_ENABLED,
13951385
SHARED_OBJECTS_DEBUG,
13961386
SHARED_OBJECTS_FORCE,
1397-
SHARED_OBJECTS_SHARE_ALL,
13981387
};
13991388
}
14001389

‎tool/generate-options.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ def parse_reference_defaults(default)
126126
@Generated("tool/generate-options.rb")
127127
public class Options {
128128
129-
<% options.each do |o| %>public final <%= o.type %> <%= o.constant %>;
129+
<% options.each do |o| %>/** --<%= o.name %>=<%= o.env_condition %><%= o.default %> */
130+
public final <%= o.type %> <%= o.constant %>;
130131
<% end %>
131132
public Options(Env env, OptionValues options) {
132133
<% options.each do |o| %> <%= o.constant %> = <%= o.env_condition %><%=

0 commit comments

Comments
 (0)