Commit 598087ef 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).

This is a retry of https://android-review.googlesource.com/#/c/88570/.
getSerializedSize() has been kept non-final so that messages generated
with a previous version of the compiler will not break.

Change-Id: I8d8154a10938cde579ae19c55eae55b1e70e0bda
parent dc2ab875
...@@ -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.
...@@ -61,8 +61,17 @@ public abstract class MessageNano { ...@@ -61,8 +61,17 @@ public abstract class MessageNano {
* using getCachedSize(). * using getCachedSize().
*/ */
public int getSerializedSize() { public 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,12 @@ public class NanoTest extends TestCase { ...@@ -105,6 +105,12 @@ 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;
assertTrue(msgSerializedSize != msg.getSerializedSize());
msg.clear();
assertEquals(0, msg.getSerializedSize());
} }
public void testRecursiveMessageNano() throws Exception { public void testRecursiveMessageNano() throws Exception {
...@@ -143,6 +149,12 @@ public class NanoTest extends TestCase { ...@@ -143,6 +149,12 @@ public class NanoTest extends TestCase {
assertEquals(3, newMsg.repeatedRecursiveMessageNano[0].id); assertEquals(3, newMsg.repeatedRecursiveMessageNano[0].id);
} }
public void testMessageNoFields() {
SingleMessageNano msg = new SingleMessageNano();
assertEquals(0, msg.getSerializedSize());
assertEquals(0, MessageNano.toByteArray(msg).length);
}
public void testNanoRequiredInt32() throws Exception { public void testNanoRequiredInt32() throws Exception {
TestAllTypesNano msg = new TestAllTypesNano(); TestAllTypesNano msg = new TestAllTypesNano();
msg.id = 123; msg.id = 123;
......
...@@ -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