Fixes memory leaks in the eventhandler

Fixes memory leaks in the eventhandler

These probably only happen when the VM is going down after an unhandled
exception, but I want to fix them anyway.

R=asiva@google.com

Committed: https://github.com/dart-lang/sdk/commit/92abe83b26aabbba4f8cfbf23dd73d9b343f10a0

[zra, 2016-08-09 18:15:50]

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

[siva, 2016-08-09 21:41:43]

lgtm

https://codereview.chromium.org/2228503007/diff/40001/runtime/bin/eventhandler.h
File runtime/bin/eventhandler.h (right):

https://codereview.chromium.org/2228503007/diff/40001/runtime/bin/eventhandle...
runtime/bin/eventhandler.h:206: RemoveHead();
Wouldn't it be more atomic to pass the clear function down to RemoveHead and
invoke it after head is removed from the list.

There seems to be no locks around this not sure if multiple threads can access
the list and see an entry is an inconsistent state because it still is in the
list but the PortEntry has been deleted.

https://codereview.chromium.org/2228503007/diff/40001/runtime/bin/socket.cc
File runtime/bin/socket.cc (right):

https://codereview.chromium.org/2228503007/diff/40001/runtime/bin/socket.cc#n...
runtime/bin/socket.cc:130: bool ListeningSocketRegistry::CloseOneSafe(OSSocket*
os_socket) {
ASSERT(mutex_ is owned by current thread);
ASSERT(os_socket != NULL);

https://codereview.chromium.org/2228503007/diff/40001/runtime/bin/socket.cc#n...
runtime/bin/socket.cc:157: 
ASSERT(os_socket->ref_count == 0);

https://codereview.chromium.org/2228503007/diff/40001/runtime/bin/socket.cc#n...
runtime/bin/socket.cc:179: return CloseOneSafe(os_socket);
why not
CloseOneSafe(it->second);

[zra, 2016-08-09 22:19:28]

https://codereview.chromium.org/2228503007/diff/40001/runtime/bin/eventhandler.h
File runtime/bin/eventhandler.h (right):

https://codereview.chromium.org/2228503007/diff/40001/runtime/bin/eventhandle...
runtime/bin/eventhandler.h:206: RemoveHead();
On 2016/08/09 21:41:43, siva wrote:
> Wouldn't it be more atomic to pass the clear function down to RemoveHead and
> invoke it after head is removed from the list.
> 
> There seems to be no locks around this not sure if multiple threads can access
> the list and see an entry is an inconsistent state because it still is in the
> list but the PortEntry has been deleted.

Done. The DescriptorInfos and therefore the CircularLinkedList of active readers
are only touched by the event handler thread, AFAIU.

https://codereview.chromium.org/2228503007/diff/40001/runtime/bin/socket.cc
File runtime/bin/socket.cc (right):

https://codereview.chromium.org/2228503007/diff/40001/runtime/bin/socket.cc#n...
runtime/bin/socket.cc:130: bool ListeningSocketRegistry::CloseOneSafe(OSSocket*
os_socket) {
On 2016/08/09 21:41:43, siva wrote:
> ASSERT(mutex_ is owned by current thread);
> ASSERT(os_socket != NULL);

For the mutexes under bin there is no "is owned by current thread", but TryLock
will apparently fail if the lock is already held.

https://codereview.chromium.org/2228503007/diff/40001/runtime/bin/socket.cc#n...
runtime/bin/socket.cc:157: 
On 2016/08/09 21:41:43, siva wrote:
> ASSERT(os_socket->ref_count == 0);

Done.

https://codereview.chromium.org/2228503007/diff/40001/runtime/bin/socket.cc#n...
runtime/bin/socket.cc:179: return CloseOneSafe(os_socket);
On 2016/08/09 21:41:43, siva wrote:
> why not
> CloseOneSafe(it->second);

Done.

[zra, 2016-08-09 22:20:53]

Description was changed from

==========
Fixes memory leaks in the eventhandler

These probably only happen when the VM is going down after an unhandled
exception, but I want to fix them anyway.
==========

to

==========
Fixes memory leaks in the eventhandler

These probably only happen when the VM is going down after an unhandled
exception, but I want to fix them anyway.

R=asiva@google.com

Committed:
https://github.com/dart-lang/sdk/commit/92abe83b26aabbba4f8cfbf23dd73d9b343f10a0
==========

[zra, 2016-08-09 22:20:55]

Committed patchset #4 (id:60001) manually as
92abe83b26aabbba4f8cfbf23dd73d9b343f10a0 (presubmit successful).