Uses an open thread handle as the ThreadJoinId on Windows.

Uses an open thread handle as the ThreadJoinId on Windows.

Also:
- Reaps exited threads in the thread pool before putting
a thread on the idle list so that a new arriving task
isn't blocked on a supposedly idle thread in the middle
of a join.
- Stops trying to join eventhandler threads on
Windows. Now that we're using the correct exit() call,
we probably don't have to worry about exit code pollution,
so joining the threads is unnecessary.

related #26400

R=asiva@google.com, iposva@google.com

Committed: https://github.com/dart-lang/sdk/commit/5f59a954f816ad0f46472eb4da05e714dbb7e117

[zra, 2016-05-16 02:59:02]

Description was changed from

==========
Use HANDLE as the ThreadJoinId on Windows.

related #26400
==========

to

==========
Use an open thread handle as the ThreadJoinId on Windows.

related #26400
==========

[zra, 2016-05-16 14:56:27]

Description was changed from

==========
Use an open thread handle as the ThreadJoinId on Windows.

related #26400
==========

to

==========
Uses an open thread handle as the ThreadJoinId on Windows.

Also, stops trying to join eventhandler threads on
Windows. Now that we're using the correct exit() call,
we probably don't have to worry about exit code pollution,
so joining the threads is unnecessary.

related #26400
==========

[zra, 2016-05-16 14:58:06]

Description was changed from

==========
Uses an open thread handle as the ThreadJoinId on Windows.

Also, stops trying to join eventhandler threads on
Windows. Now that we're using the correct exit() call,
we probably don't have to worry about exit code pollution,
so joining the threads is unnecessary.

related #26400
==========

to

==========
Uses an open thread handle as the ThreadJoinId on Windows.

Also:
- Reaps exited threads in the thread pool before putting
a thread on the idle list so that a new arriving task
isn't blocked on a supposedly idle thread in the middle
of a join.
- Stops trying to join eventhandler threads on
Windows. Now that we're using the correct exit() call,
we probably don't have to worry about exit code pollution,
so joining the threads is unnecessary.

related #26400
==========

[zra, 2016-05-16 16:05:24]

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

[zra, 2016-05-16 16:58:37]

Modified ThreadPool::SetIdleAndReapExited() as discussed offline.

[Ivan Posva, 2016-05-16 17:13:14]

https://codereview.chromium.org/1978153002/diff/40001/runtime/vm/os_thread.h
File runtime/vm/os_thread.h (right):

https://codereview.chromium.org/1978153002/diff/40001/runtime/vm/os_thread.h#...
runtime/vm/os_thread.h:210: // only once per-thread in the constructor for an
OSThread.
We should enforce this as we discussed instead of relying on the comment here.
The failure mode is pretty bad as far as I can tell.

[Ivan Posva, 2016-05-16 17:30:16]

LGTM -ip

[siva, 2016-05-16 17:59:28]

DBC

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

https://codereview.chromium.org/1978153002/diff/40001/runtime/bin/thread_win....
runtime/bin/thread_win.cc:108: }
The Thread::Join function doesn't seem to be used anymore why not just delete
it.

https://codereview.chromium.org/1978153002/diff/40001/runtime/vm/os_thread_wi...
File runtime/vm/os_thread_win.cc (right):

https://codereview.chromium.org/1978153002/diff/40001/runtime/vm/os_thread_wi...
runtime/vm/os_thread_win.cc:145: ASSERT(res == WAIT_OBJECT_0);
Are all thread guaranteed to call OSThread::Join() at some point? I only see
calls to Join from the thread pool threads, The main thread which is created by
the embedder and calls Dart_EnterIsolate will also end up with one of these
handles which never seems to be closed (leaks one handle per isolate).

https://codereview.chromium.org/1978153002/diff/40001/runtime/vm/thread_pool.cc
File runtime/vm/thread_pool.cc (right):

https://codereview.chromium.org/1978153002/diff/40001/runtime/vm/thread_pool....
runtime/vm/thread_pool.cc:215: void ThreadPool::SetIdleLocked(Worker* worker) {
ASSERT(mutex_.IsOwnedByCurrentThread());

[zra, 2016-05-16 20:32:55]

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

https://codereview.chromium.org/1978153002/diff/40001/runtime/bin/thread_win....
runtime/bin/thread_win.cc:108: }
On 2016/05/16 17:59:27, siva wrote:
> The Thread::Join function doesn't seem to be used anymore why not just delete
> it.

Done.

https://codereview.chromium.org/1978153002/diff/40001/runtime/vm/os_thread_wi...
File runtime/vm/os_thread_win.cc (right):

https://codereview.chromium.org/1978153002/diff/40001/runtime/vm/os_thread_wi...
runtime/vm/os_thread_win.cc:145: ASSERT(res == WAIT_OBJECT_0);
On 2016/05/16 17:59:27, siva wrote:
> Are all thread guaranteed to call OSThread::Join() at some point? I only see
> calls to Join from the thread pool threads, The main thread which is created
by
> the embedder and calls Dart_EnterIsolate will also end up with one of these
> handles which never seems to be closed (leaks one handle per isolate).

Changed to call GetCurrentThreadJoinId only for threads that will be joined.

https://codereview.chromium.org/1978153002/diff/40001/runtime/vm/thread_pool.cc
File runtime/vm/thread_pool.cc (right):

https://codereview.chromium.org/1978153002/diff/40001/runtime/vm/thread_pool....
runtime/vm/thread_pool.cc:215: void ThreadPool::SetIdleLocked(Worker* worker) {
On 2016/05/16 17:59:27, siva wrote:
> ASSERT(mutex_.IsOwnedByCurrentThread());

Done.

[zra, 2016-05-17 15:43:04]

ptal

[siva, 2016-05-17 17:56:51]

lgtm

[zra, 2016-05-17 20:18:17]

Description was changed from

==========
Uses an open thread handle as the ThreadJoinId on Windows.

Also:
- Reaps exited threads in the thread pool before putting
a thread on the idle list so that a new arriving task
isn't blocked on a supposedly idle thread in the middle
of a join.
- Stops trying to join eventhandler threads on
Windows. Now that we're using the correct exit() call,
we probably don't have to worry about exit code pollution,
so joining the threads is unnecessary.

related #26400
==========

to

==========
Uses an open thread handle as the ThreadJoinId on Windows.

Also:
- Reaps exited threads in the thread pool before putting
a thread on the idle list so that a new arriving task
isn't blocked on a supposedly idle thread in the middle
of a join.
- Stops trying to join eventhandler threads on
Windows. Now that we're using the correct exit() call,
we probably don't have to worry about exit code pollution,
so joining the threads is unnecessary.

related #26400

R=asiva@google.com, iposva@google.com

Committed:
https://github.com/dart-lang/sdk/commit/5f59a954f816ad0f46472eb4da05e714dbb7e117
==========

[zra, 2016-05-17 20:18:18]

Committed patchset #5 (id:80001) manually as
5f59a954f816ad0f46472eb4da05e714dbb7e117 (presubmit successful).