Fixes, remove grpc_error_free_string

14 files changed, 40 insertions, 35 deletions

diff --git a/src/core/lib/http/httpcli_security_connector.c b/src/core/lib/http/httpcli_security_connector.c
index 440817c5a6..fd6fda03c3 100644
--- a/src/core/lib/http/httpcli_security_connector.c
+++ b/src/core/lib/http/httpcli_security_connector.c
@@ -156,7 +156,7 @@ static void on_handshake_done(grpc_exec_ctx *exec_ctx, void *arg,
   if (error != GRPC_ERROR_NONE) {
     const char *msg = grpc_error_string(error);
     gpr_log(GPR_ERROR, "Secure transport setup failed: %s", msg);
-    grpc_error_free_string(msg);
+    
     c->func(exec_ctx, c->arg, NULL);
   } else {
     grpc_channel_args_destroy(exec_ctx, args->args);
diff --git a/src/core/lib/iomgr/error.c b/src/core/lib/iomgr/error.c
index 3cf644f421..ceff429fcc 100644
--- a/src/core/lib/iomgr/error.c
+++ b/src/core/lib/iomgr/error.c
@@ -190,6 +190,7 @@ static void error_destroy(grpc_error *err) {
   gpr_avl_unref(err->strs);
   gpr_avl_unref(err->errs);
   gpr_avl_unref(err->times);
+  gpr_free((void *)gpr_atm_acq_load(&err->error_string));
   gpr_free(err);
 }
 
@@ -240,6 +241,7 @@ grpc_error *grpc_error_create(const char *file, int line, const char *desc,
   err->times = gpr_avl_add(gpr_avl_create(&avl_vtable_times),
                            (void *)(uintptr_t)GRPC_ERROR_TIME_CREATED,
                            box_time(gpr_now(GPR_CLOCK_REALTIME)));
+  gpr_atm_no_barrier_store(&err->error_string, 0);
   gpr_ref_init(&err->refs, 1);
   GPR_TIMER_END("grpc_error_create", 0);
   return err;
@@ -269,6 +271,7 @@ static grpc_error *copy_error_and_unref(grpc_error *in) {
     out->strs = gpr_avl_ref(in->strs);
     out->errs = gpr_avl_ref(in->errs);
     out->times = gpr_avl_ref(in->times);
+    gpr_atm_no_barrier_store(&out->error_string, 0);
     out->next_err = in->next_err;
     gpr_ref_init(&out->refs, 1);
     GRPC_ERROR_UNREF(in);
@@ -495,7 +498,6 @@ static void add_errs(gpr_avl_node *n, char **s, size_t *sz, size_t *cap,
   *first = false;
   const char *e = grpc_error_string(n->value);
   append_str(e, s, sz, cap);
-  grpc_error_free_string(e);
   add_errs(n->right, s, sz, cap, first);
 }
 
@@ -517,7 +519,7 @@ static int cmp_kvs(const void *a, const void *b) {
   return strcmp(ka->key, kb->key);
 }
 
-static const char *finish_kvs(kv_pairs *kvs) {
+static char *finish_kvs(kv_pairs *kvs) {
   char *s = NULL;
   size_t sz = 0;
   size_t cap = 0;
@@ -538,19 +540,18 @@ static const char *finish_kvs(kv_pairs *kvs) {
   return s;
 }
 
-void grpc_error_free_string(const char *str) {
-  if (str == no_error_string) return;
-  if (str == oom_error_string) return;
-  if (str == cancelled_error_string) return;
-  gpr_free((char *)str);
-}
-
 const char *grpc_error_string(grpc_error *err) {
   GPR_TIMER_BEGIN("grpc_error_string", 0);
   if (err == GRPC_ERROR_NONE) return no_error_string;
   if (err == GRPC_ERROR_OOM) return oom_error_string;
   if (err == GRPC_ERROR_CANCELLED) return cancelled_error_string;
 
+  void *p = (void *)gpr_atm_acq_load(&err->error_string);
+  if (p != NULL) {
+    GPR_TIMER_END("grpc_error_string", 0);
+    return p;
+  }
+
   kv_pairs kvs;
   memset(&kvs, 0, sizeof(kvs));
 
@@ -563,7 +564,13 @@ const char *grpc_error_string(grpc_error *err) {
 
   qsort(kvs.kvs, kvs.num_kvs, sizeof(kv_pair), cmp_kvs);
 
-  const char *out = finish_kvs(&kvs);
+  char *out = finish_kvs(&kvs);
+
+  if (!gpr_atm_rel_cas(&err->error_string, 0, (gpr_atm)out)) {
+    gpr_free(out);
+    out = (char *)gpr_atm_no_barrier_load(&err->error_string);
+  }
+
   GPR_TIMER_END("grpc_error_string", 0);
   return out;
 }
@@ -598,7 +605,6 @@ bool grpc_log_if_error(const char *what, grpc_error *error, const char *file,
   if (error == GRPC_ERROR_NONE) return true;
   const char *msg = grpc_error_string(error);
   gpr_log(file, line, GPR_LOG_SEVERITY_ERROR, "%s: %s", what, msg);
-  grpc_error_free_string(msg);
   GRPC_ERROR_UNREF(error);
   return false;
 }
diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h
index 75ca9c4bf9..ffacdac393 100644
--- a/src/core/lib/iomgr/error.h
+++ b/src/core/lib/iomgr/error.h
@@ -145,7 +145,6 @@ typedef enum {
 #define GRPC_ERROR_CANCELLED ((grpc_error *)4)
 
 const char *grpc_error_string(grpc_error *error);
-void grpc_error_free_string(const char *str);
 
 /// Create an error - but use GRPC_ERROR_CREATE instead
 grpc_error *grpc_error_create(const char *file, int line, const char *desc,
diff --git a/src/core/lib/iomgr/error_internal.h b/src/core/lib/iomgr/error_internal.h
index 5afe9edc7f..8f49ba3992 100644
--- a/src/core/lib/iomgr/error_internal.h
+++ b/src/core/lib/iomgr/error_internal.h
@@ -46,6 +46,7 @@ struct grpc_error {
   gpr_avl times;
   gpr_avl errs;
   uintptr_t next_err;
+  gpr_atm error_string;
 };
 
 bool grpc_error_is_special(grpc_error *err);
diff --git a/src/core/lib/iomgr/tcp_client_posix.c b/src/core/lib/iomgr/tcp_client_posix.c
index 9a77c92016..a55e7bfc72 100644
--- a/src/core/lib/iomgr/tcp_client_posix.c
+++ b/src/core/lib/iomgr/tcp_client_posix.c
@@ -118,7 +118,7 @@ static void tc_on_alarm(grpc_exec_ctx *exec_ctx, void *acp, grpc_error *error) {
     const char *str = grpc_error_string(error);
     gpr_log(GPR_DEBUG, "CLIENT_CONNECT: %s: on_alarm: error=%s", ac->addr_str,
             str);
-    grpc_error_free_string(str);
+    
   }
   gpr_mu_lock(&ac->mu);
   if (ac->fd != NULL) {
@@ -178,7 +178,7 @@ static void on_writable(grpc_exec_ctx *exec_ctx, void *acp, grpc_error *error) {
     const char *str = grpc_error_string(error);
     gpr_log(GPR_DEBUG, "CLIENT_CONNECT: %s: on_writable: error=%s",
             ac->addr_str, str);
-    grpc_error_free_string(str);
+    
   }
 
   gpr_mu_lock(&ac->mu);
diff --git a/src/core/lib/iomgr/tcp_posix.c b/src/core/lib/iomgr/tcp_posix.c
index ece44978b0..402501be5e 100644
--- a/src/core/lib/iomgr/tcp_posix.c
+++ b/src/core/lib/iomgr/tcp_posix.c
@@ -181,7 +181,7 @@ static void call_read_cb(grpc_exec_ctx *exec_ctx, grpc_tcp *tcp,
     size_t i;
     const char *str = grpc_error_string(error);
     gpr_log(GPR_DEBUG, "read: error=%s", str);
-    grpc_error_free_string(str);
+    
     for (i = 0; i < tcp->incoming_buffer->count; i++) {
       char *dump = grpc_dump_slice(tcp->incoming_buffer->slices[i],
                                    GPR_DUMP_HEX | GPR_DUMP_ASCII);
@@ -435,7 +435,7 @@ static void tcp_handle_write(grpc_exec_ctx *exec_ctx, void *arg /* grpc_tcp */,
     if (grpc_tcp_trace) {
       const char *str = grpc_error_string(error);
       gpr_log(GPR_DEBUG, "write: %s", str);
-      grpc_error_free_string(str);
+      
     }
 
     grpc_closure_run(exec_ctx, cb, error);
@@ -485,7 +485,7 @@ static void tcp_write(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep,
     if (grpc_tcp_trace) {
       const char *str = grpc_error_string(error);
       gpr_log(GPR_DEBUG, "write: %s", str);
-      grpc_error_free_string(str);
+      
     }
     grpc_closure_sched(exec_ctx, cb, error);
   }
diff --git a/src/core/lib/iomgr/tcp_server_windows.c b/src/core/lib/iomgr/tcp_server_windows.c
index 97d7827461..b25783f986 100644
--- a/src/core/lib/iomgr/tcp_server_windows.c
+++ b/src/core/lib/iomgr/tcp_server_windows.c
@@ -345,7 +345,7 @@ static void on_accept(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) {
   if (error != GRPC_ERROR_NONE) {
     const char *msg = grpc_error_string(error);
     gpr_log(GPR_INFO, "Skipping on_accept due to error: %s", msg);
-    grpc_error_free_string(msg);
+    
     gpr_mu_unlock(&sp->server->mu);
     return;
   }
diff --git a/src/core/lib/iomgr/tcp_uv.c b/src/core/lib/iomgr/tcp_uv.c
index a1ee597d09..4bf2e08684 100644
--- a/src/core/lib/iomgr/tcp_uv.c
+++ b/src/core/lib/iomgr/tcp_uv.c
@@ -157,7 +157,7 @@ static void read_callback(uv_stream_t *stream, ssize_t nread,
       size_t i;
       const char *str = grpc_error_string(error);
       gpr_log(GPR_DEBUG, "read: error=%s", str);
-      grpc_error_free_string(str);
+      
       for (i = 0; i < tcp->read_slices->count; i++) {
         char *dump = grpc_dump_slice(tcp->read_slices->slices[i],
                                      GPR_DUMP_HEX | GPR_DUMP_ASCII);
diff --git a/src/core/lib/security/transport/security_handshaker.c b/src/core/lib/security/transport/security_handshaker.c
index e886cc59a0..a14fab71fd 100644
--- a/src/core/lib/security/transport/security_handshaker.c
+++ b/src/core/lib/security/transport/security_handshaker.c
@@ -123,7 +123,7 @@ static void security_handshake_failed_locked(grpc_exec_ctx *exec_ctx,
   }
   const char *msg = grpc_error_string(error);
   gpr_log(GPR_DEBUG, "Security handshake failed: %s", msg);
-  grpc_error_free_string(msg);
+  
   if (!h->shutdown) {
     // TODO(ctiller): It is currently necessary to shutdown endpoints
     // before destroying them, even if we know that there are no
diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c
index 38b6225abd..f7e7dff097 100644
--- a/src/core/lib/surface/call.c
+++ b/src/core/lib/surface/call.c
@@ -603,14 +603,14 @@ static void get_final_status(grpc_call *call,
   int i;
   for (i = 0; i < STATUS_SOURCE_COUNT; i++) {
     if (call->status[i].is_set) {
-      const char *text = grpc_error_string(call->status[i].error);
-      gpr_log(GPR_DEBUG, "%s", text);
-      grpc_error_free_string(text);
-
       grpc_status_code code;
       const char *msg = NULL;
       grpc_error_get_status(call->status[i].error, call->send_deadline, &code,
                             &msg, NULL);
+
+      gpr_log(GPR_DEBUG, "%s --> %d %s",
+              grpc_error_string(call->status[i].error), code, msg);
+
       set_value(code, set_value_user_data);
       if (details != NULL) {
         *details = grpc_slice_from_copied_string(msg);
@@ -630,7 +630,6 @@ static void set_status_from_error(grpc_exec_ctx *exec_ctx, grpc_call *call,
   const char *es = grpc_error_string(error);
   gpr_log(GPR_DEBUG, "%p[%d]: set %d[is_set=%d] to %s", call, call->is_client,
           source, call->status[source].is_set, es);
-  grpc_error_free_string(es);
 
   if (call->status[source].is_set) {
     GRPC_ERROR_UNREF(error);
diff --git a/src/core/lib/surface/completion_queue.c b/src/core/lib/surface/completion_queue.c
index 4613c9021e..d1f1cd046e 100644
--- a/src/core/lib/surface/completion_queue.c
+++ b/src/core/lib/surface/completion_queue.c
@@ -253,7 +253,7 @@ void grpc_cq_end_op(grpc_exec_ctx *exec_ctx, grpc_completion_queue *cc,
     if (grpc_trace_operation_failures && error != GRPC_ERROR_NONE) {
       gpr_log(GPR_ERROR, "Operation failed: tag=%p, error=%s", tag, errmsg);
     }
-    grpc_error_free_string(errmsg);
+    
   }
 
   storage->tag = tag;
@@ -294,7 +294,7 @@ void grpc_cq_end_op(grpc_exec_ctx *exec_ctx, grpc_completion_queue *cc,
     if (kick_error != GRPC_ERROR_NONE) {
       const char *msg = grpc_error_string(kick_error);
       gpr_log(GPR_ERROR, "Kick failed: %s", msg);
-      grpc_error_free_string(msg);
+      
       GRPC_ERROR_UNREF(kick_error);
     }
   } else {
@@ -461,7 +461,7 @@ grpc_event grpc_completion_queue_next(grpc_completion_queue *cc,
         gpr_mu_unlock(cc->mu);
         const char *msg = grpc_error_string(err);
         gpr_log(GPR_ERROR, "Completion queue next failed: %s", msg);
-        grpc_error_free_string(msg);
+        
         GRPC_ERROR_UNREF(err);
         memset(&ret, 0, sizeof(ret));
         ret.type = GRPC_QUEUE_TIMEOUT;
@@ -647,7 +647,7 @@ grpc_event grpc_completion_queue_pluck(grpc_completion_queue *cc, void *tag,
         gpr_mu_unlock(cc->mu);
         const char *msg = grpc_error_string(err);
         gpr_log(GPR_ERROR, "Completion queue next failed: %s", msg);
-        grpc_error_free_string(msg);
+        
         GRPC_ERROR_UNREF(err);
         memset(&ret, 0, sizeof(ret));
         ret.type = GRPC_QUEUE_TIMEOUT;
diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c
index b5f905bf04..c70aaa174b 100644
--- a/src/core/lib/surface/server.c
+++ b/src/core/lib/surface/server.c
@@ -453,7 +453,7 @@ static void destroy_channel(grpc_exec_ctx *exec_ctx, channel_data *chand,
   if (grpc_server_channel_trace && error != GRPC_ERROR_NONE) {
     const char *msg = grpc_error_string(error);
     gpr_log(GPR_INFO, "Disconnected client: %s", msg);
-    grpc_error_free_string(msg);
+    
   }
   GRPC_ERROR_UNREF(error);
 
diff --git a/src/core/lib/transport/connectivity_state.c b/src/core/lib/transport/connectivity_state.c
index c656d93740..2f55ffd96c 100644
--- a/src/core/lib/transport/connectivity_state.c
+++ b/src/core/lib/transport/connectivity_state.c
@@ -163,7 +163,7 @@ void grpc_connectivity_state_set(grpc_exec_ctx *exec_ctx,
     gpr_log(GPR_DEBUG, "SET: %p %s: %s --> %s [%s] error=%p %s", tracker,
             tracker->name, grpc_connectivity_state_name(tracker->current_state),
             grpc_connectivity_state_name(state), reason, error, error_string);
-    grpc_error_free_string(error_string);
+    
   }
   switch (state) {
     case GRPC_CHANNEL_INIT:
diff --git a/src/core/lib/transport/transport_op_string.c b/src/core/lib/transport/transport_op_string.c
index 8810a6bbda..9d5f9c2d6e 100644
--- a/src/core/lib/transport/transport_op_string.c
+++ b/src/core/lib/transport/transport_op_string.c
@@ -121,7 +121,7 @@ char *grpc_transport_stream_op_string(grpc_transport_stream_op *op) {
     gpr_strvec_add(&b, gpr_strdup(" "));
     const char *msg = grpc_error_string(op->cancel_error);
     gpr_asprintf(&tmp, "CANCEL:%s", msg);
-    grpc_error_free_string(msg);
+    
     gpr_strvec_add(&b, tmp);
   }
 
@@ -160,7 +160,7 @@ char *grpc_transport_op_string(grpc_transport_op *op) {
     const char *err = grpc_error_string(op->disconnect_with_error);
     gpr_asprintf(&tmp, "DISCONNECT:%s", err);
     gpr_strvec_add(&b, tmp);
-    grpc_error_free_string(err);
+    
   }
 
   if (op->goaway_error) {
@@ -168,7 +168,7 @@ char *grpc_transport_op_string(grpc_transport_op *op) {
     first = false;
     const char *msg = grpc_error_string(op->goaway_error);
     gpr_asprintf(&tmp, "SEND_GOAWAY:%s", msg);
-    grpc_error_free_string(msg);
+    
     gpr_strvec_add(&b, tmp);
   }