Commit 5090f19e authored by Dave Hawkey's avatar Dave Hawkey

Don't reset cachedSize to 0 in getSerializedSize

This avoids a race-condition when cachedSize is momentarily set to 0
for non-empty messages if multiple threads call getSerializedSize
(e.g. during serialization).

Change-Id: I15a8ded92edbf41bf1c8d787960c5bbbc8a323c5
parent 3a210924
...@@ -437,6 +437,15 @@ and the runtime overhead. An overview of Nano features: ...@@ -437,6 +437,15 @@ and the runtime overhead. An overview of Nano features:
MessageNano. MessageNano.
- The 'bytes' type translates to the Java type byte[]. - The 'bytes' type translates to the Java type byte[].
The generated messages are not thread-safe for writes, but may be
used simultaneously from multiple threads in a read-only manner.
In other words, an appropriate synchronization mechanism (such as
a ReadWriteLock) must be used to ensure that a message, its
ancestors, and descendants are not accessed by any other threads
while the message is being modified. Field reads, getter methods,
toByteArray(...), writeTo(...), getCachedSize(), and
getSerializedSize() are all considered read-only operations.
IMPORTANT: If you have fields with defaults and opt out of accessors IMPORTANT: If you have fields with defaults and opt out of accessors
How fields with defaults are serialized has changed. Because we don't How fields with defaults are serialized has changed. Because we don't
......
...@@ -47,7 +47,7 @@ public abstract class ExtendableMessageNano<M extends ExtendableMessageNano<M>> ...@@ -47,7 +47,7 @@ public abstract class ExtendableMessageNano<M extends ExtendableMessageNano<M>>
protected List<UnknownFieldData> unknownFieldData; protected List<UnknownFieldData> unknownFieldData;
@Override @Override
public int getSerializedSize() { protected int computeSerializedSize() {
int size = 0; int size = 0;
int unknownFieldCount = unknownFieldData == null ? 0 : unknownFieldData.size(); int unknownFieldCount = unknownFieldData == null ? 0 : unknownFieldData.size();
for (int i = 0; i < unknownFieldCount; i++) { for (int i = 0; i < unknownFieldCount; i++) {
...@@ -55,7 +55,6 @@ public abstract class ExtendableMessageNano<M extends ExtendableMessageNano<M>> ...@@ -55,7 +55,6 @@ public abstract class ExtendableMessageNano<M extends ExtendableMessageNano<M>>
size += CodedOutputByteBufferNano.computeRawVarint32Size(unknownField.tag); size += CodedOutputByteBufferNano.computeRawVarint32Size(unknownField.tag);
size += unknownField.bytes.length; size += unknownField.bytes.length;
} }
cachedSize = size;
return size; return size;
} }
......
...@@ -38,7 +38,7 @@ import java.io.IOException; ...@@ -38,7 +38,7 @@ import java.io.IOException;
* @author wink@google.com Wink Saville * @author wink@google.com Wink Saville
*/ */
public abstract class MessageNano { public abstract class MessageNano {
protected int cachedSize = -1; protected volatile int cachedSize = -1;
/** /**
* Get the number of bytes required to encode this message. * Get the number of bytes required to encode this message.
...@@ -60,9 +60,18 @@ public abstract class MessageNano { ...@@ -60,9 +60,18 @@ public abstract class MessageNano {
* The size is cached and the cached result can be retrieved * The size is cached and the cached result can be retrieved
* using getCachedSize(). * using getCachedSize().
*/ */
public int getSerializedSize() { public final int getSerializedSize() {
int size = computeSerializedSize();
cachedSize = size;
return size;
}
/**
* Computes the number of bytes required to encode this message. This does not update the
* cached size.
*/
protected int computeSerializedSize() {
// This is overridden if the generated message has serialized fields. // This is overridden if the generated message has serialized fields.
cachedSize = 0;
return 0; return 0;
} }
......
...@@ -105,6 +105,14 @@ public class NanoTest extends TestCase { ...@@ -105,6 +105,14 @@ public class NanoTest extends TestCase {
assertEquals(456, newMsg.d); assertEquals(456, newMsg.d);
assertEquals(2, msg.nestedMsg.bb); assertEquals(2, msg.nestedMsg.bb);
assertEquals(SimpleMessageNano.BAR, msg.defaultNestedEnum); assertEquals(SimpleMessageNano.BAR, msg.defaultNestedEnum);
msg.nestedMsg = null;
assertEquals(msgSerializedSize, msg.getCachedSize());
assertTrue(msgSerializedSize != msg.getSerializedSize());
msg.clear();
assertEquals(0, msg.getCachedSize());
assertEquals(0, msg.getSerializedSize());
} }
public void testRecursiveMessageNano() throws Exception { public void testRecursiveMessageNano() throws Exception {
...@@ -3532,6 +3540,13 @@ public class NanoTest extends TestCase { ...@@ -3532,6 +3540,13 @@ public class NanoTest extends TestCase {
assertTrue(Arrays.equals(new boolean[] {false, true, false, true}, nonPacked.bools)); assertTrue(Arrays.equals(new boolean[] {false, true, false, true}, nonPacked.bools));
} }
public void testMessageNoFields() {
SingleMessageNano msg = new SingleMessageNano();
assertEquals(0, msg.getSerializedSize());
assertEquals(0, msg.getCachedSize());
assertEquals(0, MessageNano.toByteArray(msg).length);
}
private void assertRepeatedPackablesEqual( private void assertRepeatedPackablesEqual(
NanoRepeatedPackables.NonPacked nonPacked, NanoRepeatedPackables.Packed packed) { NanoRepeatedPackables.NonPacked nonPacked, NanoRepeatedPackables.Packed packed) {
// Not using MessageNano.equals() -- that belongs to a separate test. // Not using MessageNano.equals() -- that belongs to a separate test.
......
...@@ -304,8 +304,8 @@ GenerateMessageSerializationMethods(io::Printer* printer) { ...@@ -304,8 +304,8 @@ GenerateMessageSerializationMethods(io::Printer* printer) {
printer->Print( printer->Print(
"\n" "\n"
"@Override\n" "@Override\n"
"public int getSerializedSize() {\n" "protected int computeSerializedSize() {\n"
" int size = super.getSerializedSize();\n"); " int size = super.computeSerializedSize();\n");
printer->Indent(); printer->Indent();
for (int i = 0; i < descriptor_->field_count(); i++) { for (int i = 0; i < descriptor_->field_count(); i++) {
...@@ -314,7 +314,6 @@ GenerateMessageSerializationMethods(io::Printer* printer) { ...@@ -314,7 +314,6 @@ GenerateMessageSerializationMethods(io::Printer* printer) {
printer->Outdent(); printer->Outdent();
printer->Print( printer->Print(
" cachedSize = size;\n"
" return size;\n" " return size;\n"
"}\n"); "}\n");
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment