Fix two memory leaks in SecureSocket.

runtime/bin/secure_socket.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "bin/secure_socket.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <stdio.h>
@@ skipping 82 matching lines @@
   Dart_Handle dart_this = ThrowIfError(Dart_GetNativeArgument(args, 0));
   ASSERT(Dart_IsInstance(dart_this));
   ThrowIfError(Dart_GetNativeInstanceField(
       dart_this,
       kSecurityContextNativeFieldIndex,
       reinterpret_cast<intptr_t*>(&context)));
   return context;
 }
 
 
+static void FreeSecurityContext(
+    void* isolate_data,
+    Dart_WeakPersistentHandle handle,
+    void* context_pointer) {
+  SSL_CTX* context = static_cast<SSL_CTX*>(context_pointer);
+  SSL_CTX_free(context);
+}
+
+
 static void SetSecurityContext(Dart_NativeArguments args,
                                SSL_CTX* context) {
+  const int approximate_size_of_context = 1500;
   Dart_Handle dart_this = ThrowIfError(Dart_GetNativeArgument(args, 0));
   ASSERT(Dart_IsInstance(dart_this));
   ThrowIfError(Dart_SetNativeInstanceField(
       dart_this,
       kSecurityContextNativeFieldIndex,
       reinterpret_cast<intptr_t>(context)));
+  Dart_NewWeakPersistentHandle(dart_this,
+                               context,
+                               approximate_size_of_context,
+                               FreeSecurityContext);
 }
 
 
 static X509* GetX509Certificate(Dart_NativeArguments args) {
   X509* certificate;
   Dart_Handle dart_this = ThrowIfError(Dart_GetNativeArgument(args, 0));
   ASSERT(Dart_IsInstance(dart_this));
   ThrowIfError(Dart_GetNativeInstanceField(
       dart_this,
       kX509NativeFieldIndex,
@@ skipping 185 matching lines @@
 
 
 void FUNCTION_NAME(SecurityContext_Allocate)(Dart_NativeArguments args) {
   SSLFilter::InitializeLibrary();
   SSL_CTX* context = SSL_CTX_new(TLS_method());
   SSL_CTX_set_verify(context, SSL_VERIFY_PEER, CertificateCallback);
   SSL_CTX_set_min_version(context, TLS1_VERSION);
   SSL_CTX_set_cipher_list(context, "HIGH:MEDIUM");
   SSL_CTX_set_cipher_list_tls11(context, "HIGH:MEDIUM");
   SetSecurityContext(args, context);
-  // TODO(whesse): Use WeakPersistentHandle to free the SSL_CTX
-  // when the object is GC'd.  Also free the alpn_select_cb data pointer,

[Søren Gjesse, 2015/09/02 16:24:31]

How about the alpn_select_cb data pointer mentioned in this TODO as well?

[Bill Hesse, 2015/09/03 08:47:00]

The TODO is moved below.  The issue is harder to resolve, since it uses an
internal member of the struct, and also should free it when it is reset to a
different value (but not the same value).  It is more important to get this
major leak fixed.

-  // if non-null (allocated in SetAlpnProtocolList).
 }
 
 
 int PasswordCallback(char* buf, int size, int rwflag, void* userdata) {
   char* password = static_cast<char*>(userdata);
   if (static_cast<size_t>(size) < strlen(password) + 1) {
     Log::PrintErr("Password buffer too small.\n");
     exit(1);
     // TODO(24182): Find the actual value of size passed in here, and
     // check for password length longer than this in the Dart function
@@ skipping 489 matching lines @@
       // Because it must be passed as a single void*, terminate
       // the list of (length, data) strings with a length 0 string.
       protocol_string_copy =
           static_cast<uint8_t*>(malloc(protocol_string_len + 1));
       memmove(protocol_string_copy, protocol_string, protocol_string_len);
       protocol_string_copy[protocol_string_len] = '\0';
       SSL_CTX_set_alpn_select_cb(context, AlpnCallback, protocol_string_copy);
       // TODO(whesse): If this function is called again, free the previous
       // protocol_string_copy.  It may be better to keep this as a native
       // field on the Dart object, since fetching it from the structure is
-      // not in the public api.  Also free this when the context is destroyed.
+      // not in the public api.
+      // Also free protocol_string_copy when the context is destroyed,
+      // in FreeSecurityContext()
     } else {
       // The function makes a local copy of protocol_string, which it owns.
       if (ssl != NULL) {
         ASSERT(context == NULL);
         status = SSL_set_alpn_protos(ssl, protocol_string, protocol_string_len);
       } else {
         ASSERT(context != NULL);
         ASSERT(ssl == NULL);
         status = SSL_CTX_set_alpn_protos(
             context, protocol_string, protocol_string_len);
@@ skipping 176 matching lines @@
   // TODO(24070, 24069): Implement setting the client certificate parameters,
   //   and triggering rehandshake.
 }
 
 
 void SSLFilter::Destroy() {
   if (ssl_ != NULL) {
     SSL_free(ssl_);
     ssl_ = NULL;
   }
+  if (socket_side_ != NULL) {
+    BIO_free(socket_side_);
+    socket_side_ = NULL;
+  }
   for (int i = 0; i < kNumBuffers; ++i) {
     Dart_DeletePersistentHandle(dart_buffer_objects_[i]);
     delete[] buffers_[i];
   }
   Dart_DeletePersistentHandle(string_start_);
   Dart_DeletePersistentHandle(string_length_);
   Dart_DeletePersistentHandle(handshake_complete_);
   Dart_DeletePersistentHandle(bad_certificate_callback_);
 }
 
@@ skipping 69 matching lines @@
     } else {
       if (SSL_LOG_DATA) Log::Print(
           "WriteEncrypted  BIO_read wrote %d bytes\n", bytes_processed);
     }
   }
   return bytes_processed;
 }
 
 }  // namespace bin
 }  // namespace dart