Skip to content

Commit 275fc86

Browse files
authored
Merge pull request #5460 from headius/symbol_marshal_work
Revisit symbol hash and marshal fixes from #4576
2 parents 3f8fd97 + 5e2edf4 commit 275fc86

File tree

12 files changed

+217
-155
lines changed

12 files changed

+217
-155
lines changed

core/src/main/java/org/jruby/RubyClass.java

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -271,14 +271,6 @@ public VariableAccessor getVariableAccessorForRead(String name) {
271271
return accessor;
272272
}
273273

274-
public VariableAccessorField getObjectIdAccessorField() {
275-
return variableTableManager.getObjectIdAccessorField();
276-
}
277-
278-
public VariableAccessorField getFFIHandleAccessorField() {
279-
return variableTableManager.getFFIHandleAccessorField();
280-
}
281-
282274
public VariableAccessor getFFIHandleAccessorForRead() {
283275
return variableTableManager.getFFIHandleAccessorForRead();
284276
}
@@ -287,10 +279,6 @@ public VariableAccessor getFFIHandleAccessorForWrite() {
287279
return variableTableManager.getFFIHandleAccessorForWrite();
288280
}
289281

290-
public VariableAccessorField getObjectGroupAccessorField() {
291-
return variableTableManager.getObjectGroupAccessorField();
292-
}
293-
294282
public VariableAccessor getObjectGroupAccessorForRead() {
295283
return variableTableManager.getObjectGroupAccessorForRead();
296284
}
@@ -2364,6 +2352,21 @@ public IRubyObject invoke(ThreadContext context, IRubyObject self, String name,
23642352
return method.call(context, self, this, name, arg0, arg1, arg2);
23652353
}
23662354

2355+
@Deprecated
2356+
public VariableAccessorField getObjectIdAccessorField() {
2357+
return variableTableManager.getObjectIdAccessorField();
2358+
}
2359+
2360+
@Deprecated
2361+
public VariableAccessorField getFFIHandleAccessorField() {
2362+
return variableTableManager.getFFIHandleAccessorField();
2363+
}
2364+
2365+
@Deprecated
2366+
public VariableAccessorField getObjectGroupAccessorField() {
2367+
return variableTableManager.getObjectGroupAccessorField();
2368+
}
2369+
23672370
// OBJECT STATE
23682371

23692372
protected final Ruby runtime;

core/src/main/java/org/jruby/RubyString.java

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -263,17 +263,21 @@ private void copyCodeRangeForSubstr(RubyString from, Encoding enc) {
263263
public final int scanForCodeRange() {
264264
int cr = getCodeRange();
265265
if (cr == CR_UNKNOWN) {
266-
Encoding enc = getEncoding();
267-
if (enc.minLength() > 1 && enc.isDummy()) {
268-
cr = CR_BROKEN;
269-
} else {
270-
cr = codeRangeScan(EncodingUtils.getActualEncoding(getEncoding(), value), value);
271-
}
266+
cr = scanForCodeRange(value);
272267
setCodeRange(cr);
273268
}
274269
return cr;
275270
}
276271

272+
public static int scanForCodeRange(ByteList byteList) {
273+
Encoding enc = byteList.getEncoding();
274+
if (enc.minLength() > 1 && enc.isDummy()) {
275+
return CR_BROKEN;
276+
} else {
277+
return codeRangeScan(EncodingUtils.getActualEncoding(enc, byteList), byteList);
278+
}
279+
}
280+
277281
final boolean singleByteOptimizable() {
278282
return StringSupport.isSingleByteOptimizable(this, EncodingUtils.STR_ENC_GET(this));
279283
}

core/src/main/java/org/jruby/RubyStruct.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,15 +196,26 @@ public static RubyClass newInstance(IRubyObject recv, IRubyObject[] args, Block
196196

197197
RubyArray member = runtime.newArray();
198198

199+
int argc = args.length;
199200
if (args[args.length - 1] instanceof RubyHash) {
201+
argc--;
200202
RubyHash kwArgs = args[args.length - 1].convertToHash();
201203
IRubyObject[] rets = ArgsUtil.extractKeywordArgs(runtime.getCurrentContext(), kwArgs, "keyword_init");
202204
keywordInit = rets[0].isTrue();
203205
}
204206

205-
for (int i = (name == null && !nilName) ? 0 : 1; i < args.length; i++) {
206-
if (i == args.length - 1 && args[i] instanceof RubyHash) break;
207-
member.append(runtime.newSymbol(args[i].asJavaString()));
207+
for (int i = (name == null && !nilName) ? 0 : 1; i < argc; i++) {
208+
IRubyObject arg = args[i];
209+
RubySymbol sym;
210+
if (arg instanceof RubySymbol) {
211+
sym = (RubySymbol) arg;
212+
} else if (arg instanceof RubyString) {
213+
sym = runtime.newSymbol(arg.convertToString().getByteList());
214+
} else {
215+
sym = runtime.newSymbol(arg.asJavaString());
216+
}
217+
218+
member.append(sym);
208219
}
209220

210221
RubyClass newStruct;

core/src/main/java/org/jruby/RubySymbol.java

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
package org.jruby;
3939

4040
import org.jcodings.Encoding;
41+
import org.jcodings.specific.ASCIIEncoding;
4142
import org.jcodings.specific.USASCIIEncoding;
4243
import org.jruby.anno.JRubyClass;
4344
import org.jruby.anno.JRubyMethod;
@@ -819,8 +820,22 @@ public static IRubyObject all_symbols(IRubyObject recv) {
819820
return recv.getRuntime().getSymbolTable().all_symbols();
820821
}
821822

822-
public static RubySymbol unmarshalFrom(UnmarshalStream input) throws java.io.IOException {
823-
RubySymbol result = newSymbol(input.getRuntime(), input.unmarshalString());
823+
public static RubySymbol unmarshalFrom(UnmarshalStream input, UnmarshalStream.MarshalState state) throws java.io.IOException {
824+
ByteList byteList = input.unmarshalString();
825+
826+
if (RubyString.scanForCodeRange(byteList) == CR_7BIT) {
827+
byteList.setEncoding(USASCIIEncoding.INSTANCE);
828+
}
829+
830+
// consume encoding ivar before making string
831+
if (state.isIvarWaiting()) {
832+
input.unmarshalInt(); // throw-away, always single ivar of encoding
833+
Encoding enc = input.getEncodingFromUnmarshaled(input.unmarshalObject());
834+
byteList.setEncoding(enc);
835+
state.setIvarWaiting(false);
836+
}
837+
838+
RubySymbol result = newSymbol(input.getRuntime(), byteList);
824839

825840
input.registerLinkTarget(result);
826841

@@ -1201,7 +1216,9 @@ private static int javaStringHashCode(ByteList iso8859) {
12011216

12021217
@Override
12031218
public boolean shouldMarshalEncoding() {
1204-
return getMarshalEncoding() != USASCIIEncoding.INSTANCE;
1219+
Encoding enc = getMarshalEncoding();
1220+
1221+
return enc != USASCIIEncoding.INSTANCE && enc != ASCIIEncoding.INSTANCE;
12051222
}
12061223

12071224
@Override

core/src/main/java/org/jruby/ext/stringio/StringIO.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import org.jruby.runtime.ThreadContext;
4747
import org.jruby.runtime.builtin.IRubyObject;
4848
import org.jruby.runtime.encoding.EncodingCapable;
49+
import org.jruby.runtime.marshal.DataType;
4950
import org.jruby.util.ArraySupport;
5051
import org.jruby.util.ByteList;
5152
import org.jruby.util.StringSupport;
@@ -61,7 +62,7 @@
6162
import static org.jruby.runtime.Visibility.PRIVATE;
6263

6364
@JRubyClass(name="StringIO")
64-
public class StringIO extends RubyObject implements EncodingCapable {
65+
public class StringIO extends RubyObject implements EncodingCapable, DataType {
6566
static class StringIOData {
6667
/**
6768
* ATTN: the value of internal might be reset to null

core/src/main/java/org/jruby/ext/thread/Mutex.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,13 @@
4141
import org.jruby.runtime.ObjectAllocator;
4242
import org.jruby.runtime.ThreadContext;
4343
import org.jruby.runtime.builtin.IRubyObject;
44+
import org.jruby.runtime.marshal.DataType;
4445

4546
/**
4647
* The "Mutex" class from the 'thread' library.
4748
*/
4849
@JRubyClass(name = "Mutex")
49-
public class Mutex extends RubyObject {
50+
public class Mutex extends RubyObject implements DataType {
5051
ReentrantLock lock = new ReentrantLock();
5152

5253
@JRubyMethod(name = "new", rest = true, meta = true)

core/src/main/java/org/jruby/runtime/ivars/VariableTableManager.java

Lines changed: 60 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,13 @@ public class VariableTableManager {
7070
private volatile String[] variableNames = EMPTY_STRING_ARRAY;
7171

7272
/** whether a slot has been allocated to object_id */
73-
private volatile boolean hasObjectID = false;
73+
private volatile int hasObjectID = 0;
74+
75+
/** whether a slot has been allocated to ffi */
76+
private volatile int hasFFI = 0;
77+
78+
/** whether a slot has been allocated to objectspace_group */
79+
private volatile int hasObjectspaceGroup = 0;
7480

7581
/** whether objects associated with this table use fields */
7682
private volatile int fieldVariables = 0;
@@ -106,7 +112,7 @@ public Map<String, VariableAccessor> getVariableAccessorsForRead() {
106112
* @return true if object_id has been allocated; false otherwise
107113
*/
108114
public boolean hasObjectID() {
109-
return hasObjectID;
115+
return hasObjectID == 1;
110116
}
111117

112118
/**
@@ -117,16 +123,17 @@ public boolean hasObjectID() {
117123
* @return the object's object_id (possibly new)
118124
*/
119125
public long getObjectId(RubyBasicObject self) {
120-
VariableAccessor objectIdAccessor = getObjectIdAccessorField().getVariableAccessorForRead();
126+
VariableAccessor objectIdAccessor = getObjectIdAccessorForRead();
121127
Long id = (Long)objectIdAccessor.get(self);
122128
if (id != null) return id;
123129

124130
synchronized (self) {
125-
objectIdAccessor = getObjectIdAccessorField().getVariableAccessorForRead();
131+
objectIdAccessor = getObjectIdAccessorForRead();
126132
id = (Long)objectIdAccessor.get(self);
127133
if (id != null) return id;
128134

129-
return initObjectId(self, getObjectIdAccessorField().getVariableAccessorForWrite(this));
135+
objectIdAccessor = getObjectIdAccessorForWrite();
136+
return initObjectId(self, objectIdAccessor);
130137
}
131138
}
132139

@@ -228,25 +235,28 @@ public VariableAccessor getVariableAccessorForRead(String name) {
228235
}
229236

230237
/**
231-
* Retrieve the lazy accessor (VariableAccessorField) for object_id.
238+
* Retrieve the read accessor for object_id for reads. If no object_id has been prepared, this will return a dummy
239+
* accessor that just returns null.
232240
*
233-
* @return the lazy accessor for object_id
241+
* @return the read accessor for object_id
234242
*/
235-
public VariableAccessorField getObjectIdAccessorField() {
236-
return objectIdVariableAccessorField;
243+
public VariableAccessor getObjectIdAccessorForRead() {
244+
return objectIdVariableAccessorField.getVariableAccessorForRead();
237245
}
238246

239247
/**
240-
* Retrieve the lazy accessor (VariableAccessorField) for FFI handle.
248+
* Retrieve the write accessor for object_id.
241249
*
242-
* @return the lazy accessor for FFI handle
250+
* @return the write accessor for object_id
243251
*/
244-
public VariableAccessorField getFFIHandleAccessorField() {
245-
return ffiHandleVariableAccessorField;
252+
public VariableAccessor getObjectIdAccessorForWrite() {
253+
if (hasObjectID == 0) hasObjectID = 1;
254+
return objectIdVariableAccessorField.getVariableAccessorForWrite(this);
246255
}
247256

248257
/**
249-
* Retrieve the read accessor for FFI handle.
258+
* Retrieve the read accessor for FFI handle. If no object_id has been prepared, this will return a dummy
259+
* accessor that just returns null.
250260
*
251261
* @return the read accessor for FFI handle
252262
*/
@@ -260,20 +270,13 @@ public VariableAccessor getFFIHandleAccessorForRead() {
260270
* @return the write accessor for FFI handle
261271
*/
262272
public VariableAccessor getFFIHandleAccessorForWrite() {
273+
if (hasFFI == 0) hasFFI = 1;
263274
return ffiHandleVariableAccessorField.getVariableAccessorForWrite(this);
264275
}
265276

266277
/**
267-
* Retrieve the lazy accessor (VariableAccessorField) for object group.
268-
*
269-
* @return the lazy accessor for object group
270-
*/
271-
public VariableAccessorField getObjectGroupAccessorField() {
272-
return objectGroupVariableAccessorField;
273-
}
274-
275-
/**
276-
* Retrieve the read accessor for object group.
278+
* Retrieve the read accessor for object group. If no object_id has been prepared, this will return a dummy
279+
* accessor that just returns null.
277280
*
278281
* @return the read accessor for object group
279282
*/
@@ -287,6 +290,7 @@ public VariableAccessor getObjectGroupAccessorForRead() {
287290
* @return the write accessor for object group
288291
*/
289292
public VariableAccessor getObjectGroupAccessorForWrite() {
293+
if (hasObjectspaceGroup == 0) hasObjectspaceGroup = 1;
290294
return objectGroupVariableAccessorField.getVariableAccessorForWrite(this);
291295
}
292296

@@ -424,7 +428,8 @@ public void syncVariables(RubyBasicObject self, IRubyObject other) {
424428
public boolean hasVariables(RubyBasicObject object) {
425429
// we check both to exclude object_id
426430
Object[] myVarTable;
427-
return fieldVariables > 0 || getVariableTableSize() > 0 && (myVarTable = object.varTable) != null && myVarTable.length > 0;
431+
return fieldVariables > 0 ||
432+
(myVarTable = object.varTable) != null && myVarTable.length > hasObjectID + hasFFI + hasObjectspaceGroup;
428433
}
429434

430435
public void serializeVariables(RubyBasicObject object, ObjectOutputStream oos) throws IOException {
@@ -586,4 +591,34 @@ synchronized final VariableAccessor allocateVariableAccessorForVar(String name,
586591

587592
return newVariableAccessor;
588593
}
594+
595+
/**
596+
* Retrieve the lazy accessor (VariableAccessorField) for object_id.
597+
*
598+
* @return the lazy accessor for object_id
599+
* @deprecated Use {@link #getObjectIdAccessorForRead()} or {@link #getObjectIdAccessorForWrite()}
600+
*/
601+
public VariableAccessorField getObjectIdAccessorField() {
602+
return objectIdVariableAccessorField;
603+
}
604+
605+
/**
606+
* Retrieve the lazy accessor (VariableAccessorField) for FFI handle.
607+
*
608+
* @return the lazy accessor for FFI handle
609+
* @deprecated Use {@link #getFFIHandleAccessorForRead()} or {@link #getFFIHandleAccessorForWrite()}
610+
*/
611+
public VariableAccessorField getFFIHandleAccessorField() {
612+
return ffiHandleVariableAccessorField;
613+
}
614+
615+
/**
616+
* Retrieve the lazy accessor (VariableAccessorField) for object group.
617+
*
618+
* @return the lazy accessor for object group
619+
* @deprecated Use {@link #getObjectGroupAccessorForRead()} or {@link #getObjectGroupAccessorForWrite()}
620+
*/
621+
public VariableAccessorField getObjectGroupAccessorField() {
622+
return objectGroupVariableAccessorField;
623+
}
589624
}

0 commit comments

Comments
 (0)