Merge pull request #5032 from yang-g/two_impls

Fix clean up logic when failing to add a secure port.

2 files changed, 43 insertions, 14 deletions

diff --git a/src/core/security/server_secure_chttp2.c b/src/core/security/server_secure_chttp2.c
index ee3443c828..92daab9bf1 100644
--- a/src/core/security/server_secure_chttp2.c
+++ b/src/core/security/server_secure_chttp2.c
@@ -208,6 +208,14 @@ int grpc_server_add_secure_http2_port(grpc_server *server, const char *addr,
     goto error;
   }
 
+  state->server = server;
+  state->tcp = tcp;
+  state->sc = sc;
+  state->creds = grpc_server_credentials_ref(creds);
+  state->is_shutdown = 0;
+  gpr_mu_init(&state->mu);
+  gpr_ref_init(&state->refcount, 1);
+
   for (i = 0; i < resolved->naddrs; i++) {
     port_temp = grpc_tcp_server_add_port(
         tcp, (struct sockaddr *)&resolved->addrs[i].addr,
@@ -233,14 +241,6 @@ int grpc_server_add_secure_http2_port(grpc_server *server, const char *addr,
   }
   grpc_resolved_addresses_destroy(resolved);
 
-  state->server = server;
-  state->tcp = tcp;
-  state->sc = sc;
-  state->creds = grpc_server_credentials_ref(creds);
-
-  state->is_shutdown = 0;
-  gpr_mu_init(&state->mu);
-  gpr_ref_init(&state->refcount, 1);
 
   /* Register with the server only upon success */
   grpc_server_add_listener(&exec_ctx, server, state, start, destroy);
@@ -250,17 +250,18 @@ int grpc_server_add_secure_http2_port(grpc_server *server, const char *addr,
 
 /* Error path: cleanup and return */
 error:
-  if (sc) {
-    GRPC_SECURITY_CONNECTOR_UNREF(sc, "server");
-  }
   if (resolved) {
     grpc_resolved_addresses_destroy(resolved);
   }
   if (tcp) {
     grpc_tcp_server_unref(&exec_ctx, tcp);
-  }
-  if (state) {
-    gpr_free(state);
+  } else {
+    if (sc) {
+      GRPC_SECURITY_CONNECTOR_UNREF(sc, "server");
+    }
+    if (state) {
+      gpr_free(state);
+    }
   }
   grpc_exec_ctx_finish(&exec_ctx);
   return 0;
diff --git a/test/core/surface/server_chttp2_test.c b/test/core/surface/server_chttp2_test.c
index ec7df6f0e3..0a3652a27a 100644
--- a/test/core/surface/server_chttp2_test.c
+++ b/test/core/surface/server_chttp2_test.c
@@ -32,7 +32,14 @@
  */
 
 #include <grpc/grpc.h>
+#include <grpc/grpc_security.h>
+#include <grpc/support/alloc.h>
+#include <grpc/support/host_port.h>
 #include <grpc/support/log.h>
+#include <grpc/support/time.h>
+#include "src/core/security/credentials.h"
+#include "src/core/tsi/fake_transport_security.h"
+#include "test/core/util/port.h"
 #include "test/core/util/test_config.h"
 
 void test_unparsable_target(void) {
@@ -40,10 +47,31 @@ void test_unparsable_target(void) {
   GPR_ASSERT(port == 0);
 }
 
+void test_add_same_port_twice() {
+  int port = grpc_pick_unused_port_or_die();
+  char *addr = NULL;
+  grpc_completion_queue *cq = grpc_completion_queue_create(NULL);
+  grpc_server *server = grpc_server_create(NULL, NULL);
+  grpc_server_credentials *fake_creds =
+      grpc_fake_transport_security_server_credentials_create();
+  gpr_join_host_port(&addr, "localhost", port);
+  GPR_ASSERT(grpc_server_add_secure_http2_port(server, addr, fake_creds));
+  GPR_ASSERT(grpc_server_add_secure_http2_port(server, addr, fake_creds) == 0);
+
+  grpc_server_credentials_release(fake_creds);
+  gpr_free(addr);
+  grpc_server_shutdown_and_notify(server, cq, NULL);
+  grpc_completion_queue_pluck(cq, NULL, gpr_inf_future(GPR_CLOCK_REALTIME),
+                              NULL);
+  grpc_server_destroy(server);
+  grpc_completion_queue_destroy(cq);
+}
+
 int main(int argc, char **argv) {
   grpc_test_init(argc, argv);
   grpc_init();
   test_unparsable_target();
+  test_add_same_port_twice();
   grpc_shutdown();
   return 0;
 }