Skip to content

Commit 43f92f1

Browse files
komamitsukomamitsu
committed
Fix streamWriteContext, parser ThreadLocal byte-array leak, SerializedString stubs, and Version
- MessagePackGenerator: implement streamWriteContext() using SimpleStreamWriteContext; wire writeStartArray/Object, endCurrentContainer, writeName, _verifyValueWrite, currentValue, and assignCurrentValue to track write context correctly - MessagePackParser: on close, null out the byte-array source in the ThreadLocal to release large payload references; InputStream sources are preserved to keep the sequential-read-from-same-stream behavior working - MessagePackSerializedString: implement all stub append/write/put methods - Add PackageVersion (0.9.12) and use it in generator, parser, and factory - Note: messageBufferOutputHolder ThreadLocal not fixed — clearing it causes 23% serialization regression by defeating the OutputStreamBufferOutput reuse cache
1 parent fa8f901 commit 43f92f1

8 files changed

Lines changed: 218 additions & 24 deletions

File tree

‎msgpack-jackson3/src/main/java/org/msgpack/jackson/dataformat/MessagePackFactory.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ public TokenStreamFactory snapshot()
188188
@Override
189189
public Version version()
190190
{
191-
return Version.unknownVersion();
191+
return PackageVersion.VERSION;
192192
}
193193

194194
@VisibleForTesting

‎msgpack-jackson3/src/main/java/org/msgpack/jackson/dataformat/MessagePackGenerator.java‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,13 @@
1818
import tools.jackson.core.Base64Variant;
1919
import tools.jackson.core.JacksonException;
2020
import tools.jackson.core.util.JacksonFeatureSet;
21+
import tools.jackson.core.util.SimpleStreamWriteContext;
2122
import tools.jackson.core.JsonGenerator;
2223
import tools.jackson.core.ObjectWriteContext;
2324
import tools.jackson.core.SerializableString;
2425
import tools.jackson.core.StreamWriteCapability;
2526
import tools.jackson.core.StreamWriteFeature;
27+
import tools.jackson.core.TokenStreamContext;
2628
import tools.jackson.core.base.GeneratorBase;
2729
import tools.jackson.core.io.IOContext;
2830
import tools.jackson.core.io.SerializedString;
@@ -61,6 +63,7 @@ public class MessagePackGenerator
6163
private int currentState = IN_ROOT;
6264
private final List<Node> nodes;
6365
private boolean isElementsClosed = false;
66+
private SimpleStreamWriteContext writeContext;
6467

6568
private static final class AsciiCharString
6669
{
@@ -201,6 +204,7 @@ private MessagePackGenerator(
201204
this.packerConfig = packerConfig;
202205
this.nodes = new ArrayList<>();
203206
this.supportIntegerKeys = supportIntegerKeys;
207+
this.writeContext = SimpleStreamWriteContext.createRootContext(null);
204208
}
205209

206210
public MessagePackGenerator(
@@ -219,6 +223,7 @@ public MessagePackGenerator(
219223
this.packerConfig = packerConfig;
220224
this.nodes = new ArrayList<>();
221225
this.supportIntegerKeys = supportIntegerKeys;
226+
this.writeContext = SimpleStreamWriteContext.createRootContext(null);
222227
}
223228

224229
private MessageBufferOutput getMessageBufferOutputForOutputStream(
@@ -270,6 +275,7 @@ public JsonGenerator writeStartArray(Object currentValue) throws JacksonExceptio
270275
@Override
271276
public JsonGenerator writeStartArray(Object currentValue, int size) throws JacksonException
272277
{
278+
writeContext = writeContext.createChildArrayContext(currentValue);
273279
if (currentState == IN_OBJECT) {
274280
Node node = nodes.get(nodes.size() - 1);
275281
assert node instanceof NodeEntryInObject;
@@ -309,6 +315,7 @@ public JsonGenerator writeStartObject(Object currentValue) throws JacksonExcepti
309315
@Override
310316
public JsonGenerator writeStartObject(Object forValue, int size) throws JacksonException
311317
{
318+
writeContext = writeContext.createChildObjectContext(forValue);
312319
if (currentState == IN_OBJECT) {
313320
Node node = nodes.get(nodes.size() - 1);
314321
assert node instanceof NodeEntryInObject;
@@ -335,6 +342,7 @@ public JsonGenerator writeEndObject() throws JacksonException
335342

336343
private void endCurrentContainer()
337344
{
345+
writeContext = writeContext.clearAndGetParent();
338346
Node parent = nodes.get(currentParentElementIndex);
339347
if (currentParentElementIndex == 0) {
340348
isElementsClosed = true;
@@ -553,6 +561,7 @@ public JacksonFeatureSet<StreamWriteCapability> streamWriteCapabilities()
553561
@Override
554562
public JsonGenerator writeName(String name) throws JacksonException
555563
{
564+
writeContext.writeName(name);
556565
addKeyNode(name);
557566
return this;
558567
}
@@ -953,13 +962,13 @@ private void flushMessagePacker()
953962
@Override
954963
public tools.jackson.core.Version version()
955964
{
956-
return tools.jackson.core.Version.unknownVersion();
965+
return PackageVersion.VERSION;
957966
}
958967

959968
@Override
960-
public tools.jackson.core.TokenStreamContext streamWriteContext()
969+
public TokenStreamContext streamWriteContext()
961970
{
962-
return null;
971+
return writeContext;
963972
}
964973

965974
@Override
@@ -977,12 +986,13 @@ public int streamWriteOutputBuffered()
977986
@Override
978987
public Object currentValue()
979988
{
980-
return null;
989+
return writeContext.currentValue();
981990
}
982991

983992
@Override
984993
public void assignCurrentValue(Object v)
985994
{
995+
writeContext.assignCurrentValue(v);
986996
}
987997

988998
@Override
@@ -999,6 +1009,7 @@ protected void _releaseBuffers()
9991009
@Override
10001010
protected void _verifyValueWrite(String typeMsg) throws JacksonException
10011011
{
1012+
writeContext.writeValue();
10021013
}
10031014

10041015
private MessagePacker getMessagePacker()

‎msgpack-jackson3/src/main/java/org/msgpack/jackson/dataformat/MessagePackParser.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ public void setExtensionTypeCustomDeserializers(ExtensionTypeCustomDeserializers
135135
@Override
136136
public Version version()
137137
{
138-
return Version.unknownVersion();
138+
return PackageVersion.VERSION;
139139
}
140140

141141
private String unpackString(MessageUnpacker messageUnpacker) throws IOException
@@ -577,6 +577,10 @@ public void close()
577577
}
578578
finally {
579579
isClosed = true;
580+
Tuple<Object, MessageUnpacker> tuple = messageUnpackerHolder.get();
581+
if (tuple != null && tuple.first() instanceof byte[]) {
582+
messageUnpackerHolder.set(new Tuple<>(null, tuple.second()));
583+
}
580584
}
581585
}
582586

‎msgpack-jackson3/src/main/java/org/msgpack/jackson/dataformat/MessagePackSerializedString.java‎

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import tools.jackson.core.SerializableString;
1919

20+
import java.io.IOException;
2021
import java.io.OutputStream;
2122
import java.nio.ByteBuffer;
2223
import java.nio.charset.Charset;
@@ -66,49 +67,75 @@ public byte[] asQuotedUTF8()
6667
@Override
6768
public int appendQuotedUTF8(byte[] bytes, int i)
6869
{
69-
return 0;
70+
byte[] utf8 = asQuotedUTF8();
71+
System.arraycopy(utf8, 0, bytes, i, utf8.length);
72+
return utf8.length;
7073
}
7174

7275
@Override
7376
public int appendQuoted(char[] chars, int i)
7477
{
75-
return 0;
78+
char[] q = asQuotedChars();
79+
System.arraycopy(q, 0, chars, i, q.length);
80+
return q.length;
7681
}
7782

7883
@Override
7984
public int appendUnquotedUTF8(byte[] bytes, int i)
8085
{
81-
return 0;
86+
byte[] utf8 = asUnquotedUTF8();
87+
System.arraycopy(utf8, 0, bytes, i, utf8.length);
88+
return utf8.length;
8289
}
8390

8491
@Override
8592
public int appendUnquoted(char[] chars, int i)
8693
{
87-
return 0;
94+
String v = getValue();
95+
v.getChars(0, v.length(), chars, i);
96+
return v.length();
8897
}
8998

9099
@Override
91100
public int writeQuotedUTF8(OutputStream outputStream)
92101
{
93-
return 0;
102+
try {
103+
byte[] utf8 = asQuotedUTF8();
104+
outputStream.write(utf8);
105+
return utf8.length;
106+
}
107+
catch (IOException e) {
108+
return -1;
109+
}
94110
}
95111

96112
@Override
97113
public int writeUnquotedUTF8(OutputStream outputStream)
98114
{
99-
return 0;
115+
try {
116+
byte[] utf8 = asUnquotedUTF8();
117+
outputStream.write(utf8);
118+
return utf8.length;
119+
}
120+
catch (IOException e) {
121+
return -1;
122+
}
100123
}
101124

102125
@Override
103126
public int putQuotedUTF8(ByteBuffer byteBuffer)
104127
{
105-
return 0;
128+
byte[] utf8 = asQuotedUTF8();
129+
byteBuffer.put(utf8);
130+
return utf8.length;
106131
}
107132

108133
@Override
109134
public int putUnquotedUTF8(ByteBuffer byteBuffer)
110135
{
111-
return 0;
136+
byte[] utf8 = asUnquotedUTF8();
137+
byteBuffer.put(utf8);
138+
return utf8.length;
112139
}
113140

114141
public Object getRawValue()

‎msgpack-jackson3/src/main/java/org/msgpack/jackson/dataformat/PackageVersion.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// MessagePack for Java
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
//
15+
package org.msgpack.jackson.dataformat;
16+
17+
import tools.jackson.core.Version;
18+
import tools.jackson.core.Versioned;
19+
20+
public class PackageVersion
21+
implements Versioned
22+
{
23+
public static final Version VERSION = new Version(0, 9, 12, null, "org.msgpack", "msgpack-jackson3");
24+
25+
@Override
26+
public Version version()
27+
{
28+
return VERSION;
29+
}
30+
}

‎msgpack-jackson3/src/test/java/org/msgpack/jackson/dataformat/MessagePackGeneratorTest.java‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import tools.jackson.core.JacksonException;
2222
import tools.jackson.core.ObjectWriteContext;
2323
import tools.jackson.core.StreamWriteFeature;
24+
import tools.jackson.core.TokenStreamContext;
2425
import tools.jackson.databind.ObjectMapper;
2526
import tools.jackson.databind.SerializationContext;
2627
import tools.jackson.databind.ValueSerializer;
@@ -1127,4 +1128,83 @@ public void testWriteBinaryByteBufferWithOffset()
11271128
byte[] result = unpacker.readPayload(unpacker.unpackBinaryHeader());
11281129
assertArrayEquals(new byte[] {0x01, 0x02, 0x03}, result);
11291130
}
1131+
1132+
@Test
1133+
public void testStreamWriteContext()
1134+
throws IOException
1135+
{
1136+
ByteArrayOutputStream baos = new ByteArrayOutputStream();
1137+
JsonGenerator generator = factory.createGenerator(ObjectWriteContext.empty(), baos);
1138+
1139+
TokenStreamContext ctx = generator.streamWriteContext();
1140+
assertNotEquals(null, ctx);
1141+
assertTrue(ctx.inRoot());
1142+
1143+
generator.writeStartArray();
1144+
ctx = generator.streamWriteContext();
1145+
assertTrue(ctx.inArray());
1146+
1147+
generator.writeStartObject();
1148+
ctx = generator.streamWriteContext();
1149+
assertTrue(ctx.inObject());
1150+
1151+
generator.writeName("k");
1152+
assertEquals("k", ctx.currentName());
1153+
1154+
generator.writeNumber(1);
1155+
generator.writeEndObject();
1156+
ctx = generator.streamWriteContext();
1157+
assertTrue(ctx.inArray());
1158+
1159+
generator.writeEndArray();
1160+
ctx = generator.streamWriteContext();
1161+
assertTrue(ctx.inRoot());
1162+
1163+
generator.close();
1164+
}
1165+
1166+
@Test
1167+
public void testCurrentValue()
1168+
throws IOException
1169+
{
1170+
ByteArrayOutputStream baos = new ByteArrayOutputStream();
1171+
JsonGenerator generator = factory.createGenerator(ObjectWriteContext.empty(), baos);
1172+
1173+
Object pojo = new Object();
1174+
generator.writeStartObject(pojo);
1175+
assertEquals(pojo, generator.currentValue());
1176+
generator.writeName("k");
1177+
generator.writeNumber(1);
1178+
generator.writeEndObject();
1179+
generator.close();
1180+
}
1181+
1182+
@Test
1183+
public void testVersion()
1184+
{
1185+
assertNotEquals(null, factory.version());
1186+
assertEquals("org.msgpack", factory.version().getGroupId());
1187+
assertEquals("msgpack-jackson3", factory.version().getArtifactId());
1188+
}
1189+
1190+
@Test
1191+
public void testSerializedStringMethods()
1192+
{
1193+
MessagePackSerializedString s = new MessagePackSerializedString("hello");
1194+
1195+
byte[] utf8Target = new byte[10];
1196+
int written = s.appendUnquotedUTF8(utf8Target, 2);
1197+
assertEquals(5, written);
1198+
assertArrayEquals(new byte[] {'h', 'e', 'l', 'l', 'o'}, Arrays.copyOfRange(utf8Target, 2, 7));
1199+
1200+
char[] charTarget = new char[10];
1201+
written = s.appendUnquoted(charTarget, 3);
1202+
assertEquals(5, written);
1203+
assertEquals("hello", new String(charTarget, 3, 5));
1204+
1205+
ByteArrayOutputStream baos = new ByteArrayOutputStream();
1206+
written = s.writeUnquotedUTF8(baos);
1207+
assertEquals(5, written);
1208+
assertArrayEquals("hello".getBytes(java.nio.charset.StandardCharsets.UTF_8), baos.toByteArray());
1209+
}
11301210
}

‎msgpack-jackson3/src/test/java/org/msgpack/jackson/dataformat/MessagePackParserTest.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,4 +1089,23 @@ public void handleMissingValueInMap()
10891089
objectMapper.readValue(out.toByteArray(), new TypeReference<Map<String, Integer>>() {});
10901090
});
10911091
}
1092+
1093+
@Test
1094+
public void testByteArrayThreadLocalClearedAfterClose()
1095+
throws IOException
1096+
{
1097+
ObjectMapper objectMapper = new MessagePackMapper(new MessagePackFactory());
1098+
1099+
byte[] bytes = objectMapper.writeValueAsBytes(Arrays.asList(1, 2, 3));
1100+
1101+
// Parse once; this caches the byte array in the ThreadLocal
1102+
objectMapper.readValue(bytes, new TypeReference<List<Integer>>() {});
1103+
1104+
// Parse again with the same byte array instance and AUTO_CLOSE_SOURCE enabled
1105+
// (default). The byte array reference should have been cleared from the
1106+
// ThreadLocal on close, so the second parse resets the unpacker and starts
1107+
// from the beginning rather than continuing from the end.
1108+
List<Integer> result = objectMapper.readValue(bytes, new TypeReference<List<Integer>>() {});
1109+
assertEquals(Arrays.asList(1, 2, 3), result);
1110+
}
10921111
}

0 commit comments

Comments
 (0)