Merge pull request #11005 from stanley-cheung/php-fix-memory-leaks

PHP: fixed some memory leaks

package.xml

@@ -10,7 +10,7 @@
   <email>grpc-packages@google.com</email>
   <active>yes</active>
  </lead>
- <date>2017-05-02</date>
+ <date>2017-05-05</date>
  <time>16:06:07</time>
  <version>
   <release>1.3.2RC1</release>
@@ -22,7 +22,7 @@
  </stability>
  <license>BSD</license>
  <notes>
-- gRPC Core 1.3 uptake
+- Fixed some memory leaks #9559, #10996
  </notes>
  <contents>
   <dir baseinstalldir="/" name="/">
@@ -1026,6 +1026,7 @@
     <file baseinstalldir="/" name="third_party/boringssl/ssl/tls13_server.c" role="src" />
     <file baseinstalldir="/" name="third_party/boringssl/ssl/tls_method.c" role="src" />
     <file baseinstalldir="/" name="third_party/boringssl/ssl/tls_record.c" role="src" />
+    <file name="LICENSE" role="doc" />
   </dir>
  </contents>
  <dependencies>

src/php/ext/grpc/call.c

@@ -125,7 +125,12 @@ zval *grpc_parse_metadata_array(grpc_metadata_array
       php_grpc_add_next_index_stringl(inner_array, str_val,
                                       GRPC_SLICE_LENGTH(elem->value), false);
       add_assoc_zval(array, str_key, inner_array);
+      PHP_GRPC_FREE_STD_ZVAL(inner_array);
     }
+    efree(str_key);
+#if PHP_MAJOR_VERSION >= 7
+    efree(str_val);
+#endif
   }
   return array;
 }
@@ -256,8 +261,6 @@ PHP_METHOD(Call, startBatch) {
   object_init(result);
   php_grpc_ulong index;
   zval *recv_status;
-  PHP_GRPC_MAKE_STD_ZVAL(recv_status);
-  object_init(recv_status);
   zval *value;
   zval *inner_value;
   zval *message_value;
@@ -439,7 +442,7 @@ PHP_METHOD(Call, startBatch) {
   grpc_completion_queue_pluck(completion_queue, call->wrapped,
                               gpr_inf_future(GPR_CLOCK_REALTIME), NULL);
 #if PHP_MAJOR_VERSION >= 7
-  zval recv_md;
+  zval *recv_md;
 #endif
   for (int i = 0; i < op_num; i++) {
     switch(ops[i].op) {
@@ -460,8 +463,10 @@ PHP_METHOD(Call, startBatch) {
       array = grpc_parse_metadata_array(&recv_metadata TSRMLS_CC);
       add_property_zval(result, "metadata", array);
 #else
-      recv_md = *grpc_parse_metadata_array(&recv_metadata);
-      add_property_zval(result, "metadata", &recv_md);
+      recv_md = grpc_parse_metadata_array(&recv_metadata);
+      add_property_zval(result, "metadata", recv_md);
+      zval_ptr_dtor(recv_md);
+      PHP_GRPC_FREE_STD_ZVAL(recv_md);
 #endif
       PHP_GRPC_DELREF(array);
       break;
@@ -475,12 +480,16 @@ PHP_METHOD(Call, startBatch) {
       }
       break;
     case GRPC_OP_RECV_STATUS_ON_CLIENT:
+      PHP_GRPC_MAKE_STD_ZVAL(recv_status);
+      object_init(recv_status);
 #if PHP_MAJOR_VERSION < 7
       array = grpc_parse_metadata_array(&recv_trailing_metadata TSRMLS_CC);
       add_property_zval(recv_status, "metadata", array);
 #else
-      recv_md = *grpc_parse_metadata_array(&recv_trailing_metadata);
-      add_property_zval(recv_status, "metadata", &recv_md);
+      recv_md = grpc_parse_metadata_array(&recv_trailing_metadata);
+      add_property_zval(recv_status, "metadata", recv_md);
+      zval_ptr_dtor(recv_md);
+      PHP_GRPC_FREE_STD_ZVAL(recv_md);
 #endif
       PHP_GRPC_DELREF(array);
       add_property_long(recv_status, "code", status);
@@ -489,6 +498,9 @@ PHP_METHOD(Call, startBatch) {
                                    true);
       gpr_free(status_details_text);
       add_property_zval(result, "status", recv_status);
+#if PHP_MAJOR_VERSION >= 7
+      zval_ptr_dtor(recv_status);
+#endif
       PHP_GRPC_DELREF(recv_status);
       PHP_GRPC_FREE_STD_ZVAL(recv_status);
       break;

src/php/ext/grpc/call_credentials.c

@@ -172,34 +172,54 @@ void plugin_get_metadata(void *ptr, grpc_auth_metadata_context context,
   object_init(arg);
   php_grpc_add_property_string(arg, "service_url", context.service_url, true);
   php_grpc_add_property_string(arg, "method_name", context.method_name, true);
-  zval *retval;
-  PHP_GRPC_MAKE_STD_ZVAL(retval);
+  zval *retval = NULL;
 #if PHP_MAJOR_VERSION < 7
   zval **params[1];
   params[0] = &arg;
   state->fci->params = params;
   state->fci->retval_ptr_ptr = &retval;
 #else
+  PHP_GRPC_MAKE_STD_ZVAL(retval);
   state->fci->params = arg;
   state->fci->retval = retval;
 #endif
   state->fci->param_count = 1;
 
+  PHP_GRPC_DELREF(arg);
+
   /* call the user callback function */
   zend_call_function(state->fci, state->fci_cache TSRMLS_CC);
 
   grpc_status_code code = GRPC_STATUS_OK;
   grpc_metadata_array metadata;
+  bool cleanup = true;
 
   if (Z_TYPE_P(retval) != IS_ARRAY) {
+    cleanup = false;
     code = GRPC_STATUS_INVALID_ARGUMENT;
   } else if (!create_metadata_array(retval, &metadata)) {
-    grpc_metadata_array_destroy(&metadata);
     code = GRPC_STATUS_INVALID_ARGUMENT;
   }
 
+  if (retval != NULL) {
+#if PHP_MAJOR_VERSION < 7
+    zval_ptr_dtor(&retval);
+#else
+    zval_ptr_dtor(arg);
+    zval_ptr_dtor(retval);
+    PHP_GRPC_FREE_STD_ZVAL(arg);
+    PHP_GRPC_FREE_STD_ZVAL(retval);
+#endif
+  }
+
   /* Pass control back to core */
   cb(user_data, metadata.metadata, metadata.count, code, NULL);
+  if (cleanup) {
+    for (int i = 0; i < metadata.count; i++) {
+      grpc_slice_unref(metadata.metadata[i].value);
+    }
+    grpc_metadata_array_destroy(&metadata);
+  }
 }
 
 /* Cleanup function for plugin creds API */
@@ -207,8 +227,10 @@ void plugin_destroy_state(void *ptr) {
   plugin_state *state = (plugin_state *)ptr;
   efree(state->fci);
   efree(state->fci_cache);
+#if PHP_MAJOR_VERSION < 7
   PHP_GRPC_FREE_STD_ZVAL(state->fci->params);
   PHP_GRPC_FREE_STD_ZVAL(state->fci->retval);
+#endif
   efree(state);
 }
 

templates/package.xml.template

@@ -12,7 +12,7 @@
     <email>grpc-packages@google.com</email>
     <active>yes</active>
    </lead>
-   <date>2017-05-02</date>
+   <date>2017-05-05</date>
    <time>16:06:07</time>
    <version>
     <release>${settings.php_version.php()}</release>
@@ -24,7 +24,7 @@
    </stability>
    <license>BSD</license>
    <notes>
-  - gRPC Core 1.3 uptake
+  - Fixed some memory leaks #9559, #10996
    </notes>
    <contents>
     <dir baseinstalldir="/" name="/">
@@ -42,6 +42,7 @@
       % endfor
       % endif
       % endfor
+      <file name="LICENSE" role="doc" />
     </dir>
    </contents>
    <dependencies>