1 files changed, 14 insertions, 12 deletions

diff --git a/python/google/protobuf/pyext/repeated_composite_container.cc b/python/google/protobuf/pyext/repeated_composite_container.cc
index 5874d5de..ca700580 100644
--- a/python/google/protobuf/pyext/repeated_composite_container.cc
+++ b/python/google/protobuf/pyext/repeated_composite_container.cc
@@ -61,9 +61,9 @@ namespace repeated_composite_container {
 
 // TODO(tibell): We might also want to check:
 //   GOOGLE_CHECK_NOTNULL((self)->owner.get());
-#define GOOGLE_CHECK_ATTACHED(self)             \
-  do {                                   \
-    GOOGLE_CHECK_NOTNULL((self)->message);      \
+#define GOOGLE_CHECK_ATTACHED(self)                           \
+  do {                                                 \
+    GOOGLE_CHECK_NOTNULL((self)->message);                 \
     GOOGLE_CHECK_NOTNULL((self)->parent_field_descriptor); \
   } while (0);
 
@@ -152,6 +152,8 @@ static PyObject* AddToAttached(RepeatedCompositeContainer* self,
   cmsg->message = sub_message;
   cmsg->parent = self->parent;
   if (cmessage::InitAttributes(cmsg, args, kwargs) < 0) {
+    message->GetReflection()->RemoveLast(
+        message, self->parent_field_descriptor);
     Py_DECREF(cmsg);
     return NULL;
   }
@@ -210,7 +212,7 @@ PyObject* Extend(RepeatedCompositeContainer* self, PyObject* value) {
   }
   ScopedPyObjectPtr next;
   while ((next.reset(PyIter_Next(iter.get()))) != NULL) {
-    if (!PyObject_TypeCheck(next.get(), &CMessage_Type)) {
+    if (!PyObject_TypeCheck(next.get(), CMessage_Type)) {
       PyErr_SetString(PyExc_TypeError, "Not a cmessage");
       return NULL;
     }
@@ -270,8 +272,8 @@ int AssignSubscript(RepeatedCompositeContainer* self,
   }
 
   // Delete from the underlying Message, if any.
-  if (self->parent != NULL) {
-    if (cmessage::InternalDeleteRepeatedField(self->parent,
+  if (self->message != nullptr) {
+    if (cmessage::InternalDeleteRepeatedField(self->message,
                                               self->parent_field_descriptor,
                                               slice,
                                               self->child_messages) < 0) {
@@ -484,15 +486,15 @@ static PyObject* Pop(PyObject* pself, PyObject* args) {
 }
 
 // Release field of parent message and transfer the ownership to target.
-void ReleaseLastTo(CMessage* parent,
+void ReleaseLastTo(Message* message,
                    const FieldDescriptor* field,
                    CMessage* target) {
-  GOOGLE_CHECK_NOTNULL(parent);
-  GOOGLE_CHECK_NOTNULL(field);
-  GOOGLE_CHECK_NOTNULL(target);
+  GOOGLE_CHECK(message != nullptr);
+  GOOGLE_CHECK(field != nullptr);
+  GOOGLE_CHECK(target != nullptr);
 
   CMessage::OwnerRef released_message(
-      parent->message->GetReflection()->ReleaseLast(parent->message, field));
+      message->GetReflection()->ReleaseLast(message, field));
   // TODO(tibell): Deal with proto1.
 
   target->parent = NULL;
@@ -522,7 +524,7 @@ int Release(RepeatedCompositeContainer* self) {
   for (Py_ssize_t i = size - 1; i >= 0; --i) {
     CMessage* child_cmessage = reinterpret_cast<CMessage*>(
         PyList_GET_ITEM(self->child_messages, i));
-    ReleaseLastTo(self->parent, field, child_cmessage);
+    ReleaseLastTo(message, field, child_cmessage);
   }
 
   // Detach from containing message.