Skip to content

Commit a7a6ff0

Browse files
committed
Make Float#to_s thread safe.
1 parent c160244 commit a7a6ff0

File tree

2 files changed

+48
-23
lines changed

2 files changed

+48
-23
lines changed

src/main/java/org/truffleruby/core/numeric/FloatNodes.java

Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import com.oracle.truffle.api.nodes.ExplodeLoop;
1919
import com.oracle.truffle.api.profiles.BranchProfile;
2020
import com.oracle.truffle.api.profiles.ConditionProfile;
21+
2122
import org.truffleruby.SuppressFBWarnings;
2223
import org.truffleruby.builtins.CoreMethod;
2324
import org.truffleruby.builtins.CoreMethodArrayArgumentsNode;
@@ -34,6 +35,7 @@
3435
import org.truffleruby.core.string.RubyString;
3536
import org.truffleruby.core.string.StringNodes;
3637
import org.truffleruby.core.string.StringUtils;
38+
import org.truffleruby.core.thread.RubyThread;
3739
import org.truffleruby.language.NotProvided;
3840
import org.truffleruby.language.RubyDynamicObject;
3941
import org.truffleruby.language.Visibility;
@@ -840,20 +842,6 @@ public abstract static class ToSNode extends CoreMethodArrayArgumentsNode {
840842
* still not entirely correct. JRuby seems to be correct, but their logic is tied up in their printf
841843
* implementation. Also see our FormatFloatNode, which I suspect is also deficient or under-tested. */
842844

843-
private static final DecimalFormat DF_NO_EXP;
844-
private static final DecimalFormat DF_SMALL_EXP;
845-
private static final DecimalFormat DF_LARGE_EXP;
846-
847-
static {
848-
final DecimalFormatSymbols smallExpSymbols = new DecimalFormatSymbols(Locale.ENGLISH);
849-
smallExpSymbols.setExponentSeparator("e");
850-
final DecimalFormatSymbols largeExpSymbols = new DecimalFormatSymbols(Locale.ENGLISH);
851-
largeExpSymbols.setExponentSeparator("e+");
852-
DF_NO_EXP = new DecimalFormat("0.0################");
853-
DF_SMALL_EXP = new DecimalFormat("0.0################E00", smallExpSymbols);
854-
DF_LARGE_EXP = new DecimalFormat("0.0################E00", largeExpSymbols);
855-
}
856-
857845
@Specialization(guards = "value == POSITIVE_INFINITY")
858846
protected RubyString toSPositiveInfinity(double value,
859847
@Cached("specialValueRope(POSITIVE_INFINITY)") Rope cachedRope) {
@@ -874,32 +862,35 @@ protected RubyString toSNaN(double value,
874862

875863
@Specialization(guards = "hasNoExp(value)")
876864
protected RubyString toSNoExp(double value) {
877-
return makeStringNode.executeMake(makeRopeNoExp(value), Encodings.US_ASCII, CodeRange.CR_7BIT);
865+
return makeStringNode.executeMake(makeRopeNoExp(value, getLanguage().getCurrentThread()),
866+
Encodings.US_ASCII, CodeRange.CR_7BIT);
878867
}
879868

880869
@Specialization(guards = "hasLargeExp(value)")
881870
protected RubyString toSLargeExp(double value) {
882-
return makeStringNode.executeMake(makeRopeLargeExp(value), Encodings.US_ASCII, CodeRange.CR_7BIT);
871+
return makeStringNode.executeMake(makeRopeLargeExp(value, getLanguage().getCurrentThread()),
872+
Encodings.US_ASCII, CodeRange.CR_7BIT);
883873
}
884874

885875
@Specialization(guards = "hasSmallExp(value)")
886876
protected RubyString toSSmallExp(double value) {
887-
return makeStringNode.executeMake(makeRopeSmallExp(value), Encodings.US_ASCII, CodeRange.CR_7BIT);
877+
return makeStringNode.executeMake(makeRopeSmallExp(value, getLanguage().getCurrentThread()),
878+
Encodings.US_ASCII, CodeRange.CR_7BIT);
888879
}
889880

890881
@TruffleBoundary
891-
private String makeRopeNoExp(double value) {
892-
return DF_NO_EXP.format(value);
882+
private String makeRopeNoExp(double value, RubyThread thread) {
883+
return getNoExpFormat(thread).format(value);
893884
}
894885

895886
@TruffleBoundary
896-
private String makeRopeSmallExp(double value) {
897-
return DF_SMALL_EXP.format(value);
887+
private String makeRopeSmallExp(double value, RubyThread thread) {
888+
return getSmallExpFormat(thread).format(value);
898889
}
899890

900891
@TruffleBoundary
901-
private String makeRopeLargeExp(double value) {
902-
return DF_LARGE_EXP.format(value);
892+
private String makeRopeLargeExp(double value, RubyThread thread) {
893+
return getLargeExpFormat(thread).format(value);
903894
}
904895

905896
protected static boolean hasNoExp(double value) {
@@ -920,6 +911,33 @@ protected static boolean hasSmallExp(double value) {
920911
protected static Rope specialValueRope(double value) {
921912
return RopeOperations.encodeAscii(Double.toString(value), Encodings.US_ASCII.jcoding);
922913
}
914+
915+
private DecimalFormat getNoExpFormat(RubyThread thread) {
916+
if (thread.noExpFormat == null) {
917+
final DecimalFormatSymbols noExpSymbols = new DecimalFormatSymbols(Locale.ENGLISH);
918+
thread.noExpFormat = new DecimalFormat("0.0################", noExpSymbols);
919+
}
920+
return thread.noExpFormat;
921+
}
922+
923+
private DecimalFormat getSmallExpFormat(RubyThread thread) {
924+
if (thread.smallExpFormat == null) {
925+
final DecimalFormatSymbols smallExpSymbols = new DecimalFormatSymbols(Locale.ENGLISH);
926+
smallExpSymbols.setExponentSeparator("e");
927+
thread.smallExpFormat = new DecimalFormat("0.0################E00", smallExpSymbols);
928+
}
929+
return thread.smallExpFormat;
930+
}
931+
932+
private DecimalFormat getLargeExpFormat(RubyThread thread) {
933+
if (thread.largeExpFormat == null) {
934+
final DecimalFormatSymbols largeExpSymbols = new DecimalFormatSymbols(Locale.ENGLISH);
935+
largeExpSymbols.setExponentSeparator("e+");
936+
thread.largeExpFormat = new DecimalFormat("0.0################E00", largeExpSymbols);
937+
}
938+
return thread.largeExpFormat;
939+
}
940+
923941
}
924942

925943
@NonStandard

src/main/java/org/truffleruby/core/thread/RubyThread.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
package org.truffleruby.core.thread;
1111

1212
import java.lang.invoke.VarHandle;
13+
import java.text.DecimalFormat;
1314
import java.util.ArrayList;
1415
import java.util.List;
1516
import java.util.Set;
@@ -66,6 +67,12 @@ public final class RubyThread extends RubyDynamicObject implements ObjectGraphNo
6667
public String sourceLocation;
6768
Object name = Nil.INSTANCE;
6869

70+
// Decimal formats are not thread safe, so we'll create them on the thread as we need them.
71+
72+
public DecimalFormat noExpFormat;
73+
public DecimalFormat smallExpFormat;
74+
public DecimalFormat largeExpFormat;
75+
6976
public RubyThread(
7077
RubyClass rubyClass,
7178
Shape shape,

0 commit comments

Comments
 (0)