Fix memory leaks in BoringSSL secure socket implementation

Fix memory leaks in BoringSSL secure socket implementation

After running our tests with ASAN and inspecting the
BoringSSL code, it appears that the calls indicated
in the CL increment the reference count of the
objects passed in being added to the security context.
Therefore, to avoid a leak, we have to free() the
objects to decrement the reference count so that the
memory is freed when the context is destroyed.

R=asiva@google.com

Committed: https://github.com/dart-lang/sdk/commit/8808f853e37b72acbb2a705bb013ecbd6fbb8582

[zra, 2016-08-03 22:44:14]

Description was changed from

==========
Fix memory leaks in BoringSSL secure socket implementation
==========

to

==========
Fix memory leaks in BoringSSL secure socket implementation

After running our tests with ASAN and inspecting the
BoringSSL code, it appears that the calls indicated
in the CL increment the reference count of the
objects passed in being added to the security context.
Therefore, to avoid a leak, we have to free() the
objects to decrement the reference count so that the
memory is freed when the context is destroyed.
==========

[zra, 2016-08-03 22:44:56]

zra@google.com changed reviewers:
+ asiva@google.com, whesse@google.com

[siva, 2016-08-04 17:25:03]

It is weird that the documentation of these calls does not specify clearly when
the ref counts need to be decremented.
Maybe you could add comments in the code stating when the ref count is
incremented so that it clear for somebody reading this code how this works in
terms of ref counting the cert object.

LGTM.

https://codereview.chromium.org/2206233003/diff/20001/runtime/bin/secure_sock...
File runtime/bin/secure_socket_boringssl.cc (right):

https://codereview.chromium.org/2206233003/diff/20001/runtime/bin/secure_sock...
runtime/bin/secure_socket_boringssl.cc:955: X509_free(cert);
Can you add a comment that SSL_CTX_add_client_CA increments the ref count on the
cert objects and so we unconditionally decrement the ref count here.

(Maybe a ditto comment on all the spots above where you decrement the ref count
unconditionally).

[zra, 2016-08-04 17:49:34]

https://codereview.chromium.org/2206233003/diff/20001/runtime/bin/secure_sock...
File runtime/bin/secure_socket_boringssl.cc (right):

https://codereview.chromium.org/2206233003/diff/20001/runtime/bin/secure_sock...
runtime/bin/secure_socket_boringssl.cc:955: X509_free(cert);
On 2016/08/04 17:25:03, siva wrote:
> Can you add a comment that SSL_CTX_add_client_CA increments the ref count on
the
> cert objects and so we unconditionally decrement the ref count here.
> 
> (Maybe a ditto comment on all the spots above where you decrement the ref
count
> unconditionally).

Done.

[zra, 2016-08-04 17:49:54]

Description was changed from

==========
Fix memory leaks in BoringSSL secure socket implementation

After running our tests with ASAN and inspecting the
BoringSSL code, it appears that the calls indicated
in the CL increment the reference count of the
objects passed in being added to the security context.
Therefore, to avoid a leak, we have to free() the
objects to decrement the reference count so that the
memory is freed when the context is destroyed.
==========

to

==========
Fix memory leaks in BoringSSL secure socket implementation

After running our tests with ASAN and inspecting the
BoringSSL code, it appears that the calls indicated
in the CL increment the reference count of the
objects passed in being added to the security context.
Therefore, to avoid a leak, we have to free() the
objects to decrement the reference count so that the
memory is freed when the context is destroyed.

R=asiva@google.com

Committed:
https://github.com/dart-lang/sdk/commit/8808f853e37b72acbb2a705bb013ecbd6fbb8582
==========

[zra, 2016-08-04 17:49:55]

Committed patchset #3 (id:40001) manually as
8808f853e37b72acbb2a705bb013ecbd6fbb8582 (presubmit successful).