Reapply "Migrate Bio to RefCounted."

This reverts commit 75186f6b2aa6c18b9ae91923eaa005657be33c9b. This
relands the change with two changes:

1. The return value of BIO_free is preserved.

2. We call BIO_pop before destroying the BIO, not after. This is a
   no-op, but is less risky given the history of this code. In OpenSSL,
   BIO_pop notifies the BIO_METHOD with BIO_CTRL_POP. We don't currently
   have this but, if we ever do, let's not risk some big mishap.

Bug: 485657226
Change-Id: I4c55566d3325e95825b003855911f56fdd3f5a24
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/89709
Reviewed-by: Xiangfei Ding <xfding@google.com>
Commit-Queue: Xiangfei Ding <xfding@google.com>
Auto-Submit: David Benjamin <davidben@google.com>

crypto/bio/bio.cc

diff --git a/crypto/bio/bio.cc b/crypto/bio/bio.cc
index be6f65b..c2a825f 100644
--- a/crypto/bio/bio.cc
+++ b/crypto/bio/bio.cc
@@ -33,54 +33,43 @@
 static CRYPTO_EX_DATA_CLASS g_ex_data_class =
     CRYPTO_EX_DATA_CLASS_INIT_WITH_APP_DATA;
 
+Bio::Bio(const BIO_METHOD *m) : RefCounted(CheckSubClass()), method(m) {
+  CRYPTO_new_ex_data(&ex_data);
+}
+
 BIO *BIO_new(const BIO_METHOD *method) {
-  Bio *ret = NewZeroed<Bio>();
+  UniquePtr<Bio> ret(New<Bio>(method));
   if (ret == nullptr) {
     return nullptr;
   }
 
-  ret->method = method;
-  ret->shutdown = 1;
-  ret->references = 1;
-  CRYPTO_new_ex_data(&ret->ex_data);
-
-  if (method->create != nullptr && !method->create(ret)) {
-    Delete(ret);
+  if (method->create != nullptr && !method->create(ret.get())) {
     return nullptr;
   }
 
-  return ret;
+  return ret.release();
+}
+
+Bio::~Bio() {
+  BIO *next = BIO_pop(this);
+  if (method != nullptr && method->destroy != nullptr) {
+    method->destroy(this);
+  }
+  CRYPTO_free_ex_data(&g_ex_data_class, &ex_data);
+  BIO_free(next);
 }
 
 int BIO_free(BIO *bio) {
-  auto *impl = FromOpaque(bio);
-  bool is_input = true;
-  Bio *next_bio;
-  for (; impl != nullptr; impl = next_bio) {
-    if (!CRYPTO_refcount_dec_and_test_zero(&impl->references)) {
-      // |impl| was shared. If |impl| was the input, report to the caller
-      // that the object was shared. If it was a downstream reference, the input
-      // was unshared and we continue to return that |bio| was freed.
-      return !is_input;
-    }
-
-    next_bio = FromOpaque(BIO_pop(impl));
-
-    if (impl->method != nullptr && impl->method->destroy != nullptr) {
-      impl->method->destroy(impl);
-    }
-
-    CRYPTO_free_ex_data(&g_ex_data_class, &impl->ex_data);
-    Delete(impl);
-    is_input = false;
+  if (bio == nullptr) {
+    return 1;
   }
-  return 1;
+  auto *impl = FromOpaque(bio);
+  return impl->DecRefInternal();
 }
 
 int BIO_up_ref(BIO *bio) {
   auto *impl = FromOpaque(bio);
-
-  CRYPTO_refcount_inc(&impl->references);
+  impl->UpRefInternal();
   return 1;
 }
 

crypto/bio/internal.h

diff --git a/crypto/bio/internal.h b/crypto/bio/internal.h
index 6c9e02e..77cd8a8 100644
--- a/crypto/bio/internal.h
+++ b/crypto/bio/internal.h
@@ -20,6 +20,7 @@
 #include <openssl/ex_data.h>
 
 #include "../internal.h"
+#include "../mem_internal.h"
 
 #if !defined(OPENSSL_NO_SOCK)
 #if !defined(OPENSSL_WINDOWS)
@@ -52,8 +53,10 @@
 
 BSSL_NAMESPACE_BEGIN
 
-class Bio : public bio_st {
+class Bio : public bio_st, public RefCounted<Bio> {
  public:
+  explicit Bio(const BIO_METHOD *m);
+
   const BIO_METHOD *method;
   CRYPTO_EX_DATA ex_data;
 
@@ -61,23 +64,26 @@
   // integrated into |flags|, to save memory.
 
   // init is non-zero if this |BIO| has been initialised.
-  int init;
+  int init = 0;
   // shutdown is often used by specific |BIO_METHOD|s to determine whether
   // they own some underlying resource. This flag can often be controlled by
   // |BIO_set_close|. For example, whether an fd BIO closes the underlying fd
   // when it, itself, is closed.
-  int shutdown;
-  int flags;
-  int retry_reason;
+  int shutdown = 1;
+  int flags = 0;
+  int retry_reason = 0;
   // num is a BIO-specific value. For example, in fd BIOs it's used to store a
   // file descriptor.
-  int num;
-  bssl::CRYPTO_refcount_t references;
-  void *ptr;
+  int num = 0;
+  void *ptr = nullptr;
   // next_bio points to the next |BIO| in a chain. This |BIO| owns a reference
   // to |next_bio|.
-  Bio *next_bio;  // used by filter BIOs
-  uint64_t num_read, num_write;
+  Bio *next_bio = nullptr;  // used by filter BIOs
+  uint64_t num_read = 0, num_write = 0;
+
+ private:
+  friend RefCounted;
+  ~Bio();
 };
 
 #if !defined(OPENSSL_NO_SOCK)

crypto/mem_internal.h

diff --git a/crypto/mem_internal.h b/crypto/mem_internal.h
index 14254ec..5cc5e5c 100644
--- a/crypto/mem_internal.h
+++ b/crypto/mem_internal.h
@@ -139,14 +139,17 @@
 
   // These methods are intentionally named differently from `bssl::UpRef` to
   // avoid a collision. Only the implementations of `FOO_up_ref` and `FOO_free`
-  // should call these.
+  // should call these. |DecRefInternal| returns true if the object was freed
+  // and false if there are still references.
   void UpRefInternal() { CRYPTO_refcount_inc(&references_); }
-  void DecRefInternal() {
+  bool DecRefInternal() {
     if (CRYPTO_refcount_dec_and_test_zero(&references_)) {
       Derived *d = static_cast<Derived *>(this);
       d->~Derived();
       OPENSSL_free(d);
+      return true;
     }
+    return false;
   }
 
  protected: