Skip free callbacks on empty CRYPTO_EX_DATAs.

Avoids bouncing on the lock, but it doesn't really matter since it's all
taking read locks. If we're declaring that callbacks don't get to see
every object being created, they shouldn't see every object being
destroyed.

CRYPTO_dup_ex_data also already had this optimization, though it wasn't
documented.

BUG=391192

Change-Id: I5b8282335112bca3850a7c0168f8bd7f7d4a2d57
Reviewed-on: https://boringssl-review.googlesource.com/6626
Reviewed-by: Adam Langley <agl@google.com>

crypto/ex_data.c

@@ -233,19 +233,18 @@ void CRYPTO_new_ex_data(CRYPTO_EX_DATA *ad) {
 
 int CRYPTO_dup_ex_data(CRYPTO_EX_DATA_CLASS *ex_data_class, CRYPTO_EX_DATA *to,
                        const CRYPTO_EX_DATA *from) {
-  STACK_OF(CRYPTO_EX_DATA_FUNCS) *func_pointers;
-  size_t i;
-
-  if (!from->sk) {
+  if (from->sk == NULL) {
     /* In this case, |from| is blank, which is also the initial state of |to|,
      * so there's nothing to do. */
     return 1;
   }
 
+  STACK_OF(CRYPTO_EX_DATA_FUNCS) *func_pointers;
   if (!get_func_pointers(&func_pointers, ex_data_class)) {
     return 0;
   }
 
+  size_t i;
   for (i = 0; i < sk_CRYPTO_EX_DATA_FUNCS_num(func_pointers); i++) {
     CRYPTO_EX_DATA_FUNCS *func_pointer =
         sk_CRYPTO_EX_DATA_FUNCS_value(func_pointers, i);
@@ -264,13 +263,18 @@ int CRYPTO_dup_ex_data(CRYPTO_EX_DATA_CLASS *ex_data_class, CRYPTO_EX_DATA *to,
 
 void CRYPTO_free_ex_data(CRYPTO_EX_DATA_CLASS *ex_data_class, void *obj,
                          CRYPTO_EX_DATA *ad) {
-  STACK_OF(CRYPTO_EX_DATA_FUNCS) *func_pointers;
-  size_t i;
+  if (ad->sk == NULL) {
+    /* Nothing to do. */
+    return;
+  }
 
+  STACK_OF(CRYPTO_EX_DATA_FUNCS) *func_pointers;
   if (!get_func_pointers(&func_pointers, ex_data_class)) {
+    /* TODO(davidben): This leaks memory on malloc error. */
     return;
   }
 
+  size_t i;
   for (i = 0; i < sk_CRYPTO_EX_DATA_FUNCS_num(func_pointers); i++) {
     CRYPTO_EX_DATA_FUNCS *func_pointer =
         sk_CRYPTO_EX_DATA_FUNCS_value(func_pointers, i);

include/openssl/ex_data.h

@@ -161,17 +161,18 @@ OPENSSL_EXPORT void *TYPE_get_ex_data(const TYPE *t, int index);
 /* Callback types. */
 
 /* CRYPTO_EX_free is a callback function that is called when an object of the
- * class is being destroyed. For example, if this callback has been passed to
- * |SSL_get_ex_new_index| then it'll be called each time an |SSL*| is destroyed.
+ * class with extra data pointers is being destroyed. For example, if this
+ * callback has been passed to |SSL_get_ex_new_index| then it may be called each
+ * time an |SSL*| is destroyed.
  *
  * The callback is passed the new object (i.e. the |SSL*|) in |parent|. The
  * arguments |argl| and |argp| contain opaque values that were given to
  * |CRYPTO_get_ex_new_index|. The callback should return one on success, but
  * the value is ignored.
  *
- * If |CRYPTO_get_ex_new_index| was called after the creation of objects of the
- * class that this applies to then, when those those objects are destroyed,
- * this callback will be called with a NULL value for |ptr|. */
+ * This callback may be called with a NULL value for |ptr| if |parent| has no
+ * value set for this index. However, the callbacks may also be skipped entirely
+ * if no extra data pointers are set on |parent| at all. */
 typedef void CRYPTO_EX_free(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
                             int index, long argl, void *argp);
 
@@ -181,9 +182,9 @@ typedef void CRYPTO_EX_free(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
  * original object. When the callback returns, |*from_d| will be set as the
  * data for this index in |to|.
  *
- * If |CRYPTO_get_ex_new_index| was called after the creation of objects of the
- * class that this applies to then, when those those objects are copies, this
- * callback will be called with a NULL value for |*from_d|. */
+ * This callback may be called with a NULL value for |*from_d| if |from| has no
+ * value set for this index. However, the callbacks may also be skipped entirely
+ * if no extra data pointers are set on |from| at all. */
 typedef int CRYPTO_EX_dup(CRYPTO_EX_DATA *to, const CRYPTO_EX_DATA *from,
                           void **from_d, int index, long argl, void *argp);