From c82101204dcde798f870d95e91f5483c3e57eb29 Mon Sep 17 00:00:00 2001
From: Dave Hawkey <dhawkey@google.com>
Date: Thu, 20 Mar 2014 10:55:41 -0600
Subject: 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
---
 java/README.txt                                         |  9 +++++++++
 .../com/google/protobuf/nano/ExtendableMessageNano.java |  3 +--
 .../main/java/com/google/protobuf/nano/MessageNano.java | 17 +++++++++++++----
 java/src/test/java/com/google/protobuf/NanoTest.java    | 12 ++++++++++++
 4 files changed, 35 insertions(+), 6 deletions(-)

(limited to 'java')

diff --git a/java/README.txt b/java/README.txt
index 13865f6..f958d14 100644
--- a/java/README.txt
+++ b/java/README.txt
@@ -437,6 +437,15 @@ and the runtime overhead. An overview of Nano features:
   MessageNano.
 - 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
 
 How fields with defaults are serialized has changed. Because we don't
diff --git a/java/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java b/java/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java
index 839f21c..63c8afc 100644
--- a/java/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java
+++ b/java/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java
@@ -47,7 +47,7 @@ public abstract class ExtendableMessageNano<M extends ExtendableMessageNano<M>>
     protected List<UnknownFieldData> unknownFieldData;
 
     @Override
-    public int getSerializedSize() {
+    protected int computeSerializedSize() {
         int size = 0;
         int unknownFieldCount = unknownFieldData == null ? 0 : unknownFieldData.size();
         for (int i = 0; i < unknownFieldCount; i++) {
@@ -55,7 +55,6 @@ public abstract class ExtendableMessageNano<M extends ExtendableMessageNano<M>>
             size += CodedOutputByteBufferNano.computeRawVarint32Size(unknownField.tag);
             size += unknownField.bytes.length;
         }
-        cachedSize = size;
         return size;
     }
 
diff --git a/java/src/main/java/com/google/protobuf/nano/MessageNano.java b/java/src/main/java/com/google/protobuf/nano/MessageNano.java
index 82dc6cc..d6288c9 100644
--- a/java/src/main/java/com/google/protobuf/nano/MessageNano.java
+++ b/java/src/main/java/com/google/protobuf/nano/MessageNano.java
@@ -38,7 +38,7 @@ import java.io.IOException;
  * @author wink@google.com Wink Saville
  */
 public abstract class MessageNano {
-    protected int cachedSize = -1;
+    protected volatile int cachedSize = -1;
 
     /**
      * Get the number of bytes required to encode this message.
@@ -61,9 +61,18 @@ public abstract class MessageNano {
      * using getCachedSize().
      */
     public int getSerializedSize() {
-        // This is overridden if the generated message has serialized fields.
-        cachedSize = 0;
-        return 0;
+        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.
+      return 0;
     }
 
     /**
diff --git a/java/src/test/java/com/google/protobuf/NanoTest.java b/java/src/test/java/com/google/protobuf/NanoTest.java
index 9987cac..5f28647 100644
--- a/java/src/test/java/com/google/protobuf/NanoTest.java
+++ b/java/src/test/java/com/google/protobuf/NanoTest.java
@@ -105,6 +105,12 @@ public class NanoTest extends TestCase {
     assertEquals(456, newMsg.d);
     assertEquals(2, msg.nestedMsg.bb);
     assertEquals(SimpleMessageNano.BAR, msg.defaultNestedEnum);
+
+    msg.nestedMsg = null;
+    assertTrue(msgSerializedSize != msg.getSerializedSize());
+
+    msg.clear();
+    assertEquals(0, msg.getSerializedSize());
   }
 
   public void testRecursiveMessageNano() throws Exception {
@@ -143,6 +149,12 @@ public class NanoTest extends TestCase {
     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 {
     TestAllTypesNano msg = new TestAllTypesNano();
     msg.id = 123;
-- 
cgit v1.1