This reland commit 5a32d8bc7c with a fix
for thread leak (Issue #56717): when `ThreadPool` is shutting down
asynchronously the last worker should detach itself to prevent
leaking associated low-level data structures, because no thread will
join it.
A hang in service isolate shutdown (caused by an existing bug) was fixed by commit 157a0dc7f9.
This CL turns native ports into a thin abstraction over underlying
thread pool instead of building them as full fledged MessageHandler.
This allows to easily implement a variation of native ports which can
handle messages concurrently with the given degree of concurrency.
This type of port can be used to greatly simplify implementation of
IOService - which previously had to do its own concurrency management
on top of "single threaded" native ports. This capability is exposed
as `Dart_NewConcurrentNativePort` API.
The new implementation is in general much cleaner then the old one
with one exception: `Dart_CloseNativePort` API has unfortunate design
where underlying message handler is destroyed asynchronously and
`Dart_CloseNativePort` returns immediately without waiting for pending
tasks to complete. Implementing this on top of `ThreadPool` requires
some changes to thread pool implementation.
Issue https://github.com/dart-lang/sdk/issues/55844
Closes https://github.com/dart-lang/sdk/issues/56717
TEST=ci
Change-Id: Ic68bfb60757685afd75c80a70cdec66cc13c149b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/385000
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
This CL also removes ability to assign names to mutexes which was
added in c25ebfff96 but did not yield
any interesting data.
TEST=ci
CoreLibraryReviewExempt: Changes to dart:concurrent only.
Cq-Include-Trybots: luci.dart.try:vm-fuchsia-release-x64-try,vm-win-debug-x64-try,vm-win-release-x64-try,vm-aot-linux-product-x64-try
Change-Id: Id41e1d29832f6008e02f0a571ee67564e1a84224
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/375300
Reviewed-by: Martin Kustermann <kustermann@google.com>
Some fields of `OSThread` are initialized by retrieving information by
calling OS functions. For example, `trace_id_` is initialized by calling
`OSThread::GetCurrentThreadTraceId()`. Similarly, this CL adds the
`OSThread::GetCurrentThreadName()` method and uses it to initialize
`name_`.
TEST=CI
Change-Id: Iee121d71e660be01ff684a298144fa7d59e3b61d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/278662
Commit-Queue: Derek Xu <derekx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
pthread_setname_np ignores names that are too long rather than truncating. This was causing most dart:io threads to remain unnamed.
Cf. eec49f34c4.
TEST="thread info" in gdb
Change-Id: I12f47b06b81f1204f742d829e14784d792a4fb96
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/215800
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
This relands https://dart-review.googlesource.com/c/sdk/+/205633
but without renaming TARGET_OS_IPHONE to DART_TARGET_OS_IPHONE.
It also changes uses of TARGET_OS_IOS to
DART_TARGET_OS_MACOS_IOS to be consistent with the rest of the
VM.
TargetConditionals.h for XCode 13 defines several
TARGET_OS_* preprocessor symbols that confuse the
Dart build. There is probably a more targeted fix
for this, but renaming the symbols that Dart uses
will also prevent this problem if more symbols
are added to the platform headers in the future.
See: https://github.com/dart-lang/sdk/issues/46499
TEST=It builds.
Change-Id: Ie775c19dd23cfdf5f65e5ebc6ee4ec3a561676fa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205860
Commit-Queue: Zach Anderson <zra@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
TargetConditionals.h for XCode 13 defines several
TARGET_OS_* preprocessor symbols that confuse the
Dart build. There is probably a more targeted fix
for this, but renaming the symbols that Dart uses
will also prevent this problem if more symbols
are added to the platform headers in the future.
See: https://github.com/dart-lang/sdk/issues/46499
TEST=It builds.
Change-Id: I3b33a03b4a9a14b76d55fe12f8cdefec4b3c3664
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205633
Commit-Queue: Zach Anderson <zra@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
The newly added --worker-thread-priority can be used by users as an
opt-in to set the thread priority / nice value.
It is the responsibility of the user to ensure that the process has the
right to change the priority to the given value. Failure to set the
priority will be fatal.
See b/154918152
Change-Id: I3b3791e88b6ddf4fa6e39e4587f7fe1ab4d2312f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/150560
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
On POSIX platforms, execvp was being run wrapped in
`VOID_TEMP_FAILURE_RETRY` which blocked SIGPROF, resulting in SIGPROF
being blocked in the new application. This would cause Dart applications
spawned through dart:io as a new process to not receive profiler ticks.
This should fix the issue where `flutter run -d {linux,macos}` would
result in profiling being broken for the spawned desktop application.
Change-Id: I7b176ce48144bcbca29d4ae63c82599623ce3c35
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/127451
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
On Linux it is an error for a thread name to exceed 16 bytes. Thus also changing the VM thread pool to use the shorter "DartWorker" name.
Change-Id: I4c5cc2bfb831a5593a8652d6435631b9d3803720
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/126163
Commit-Queue: Kirill Nikolaev <cyriln@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
The safestack stack pointer is cached when invoking Dart code, and
manually restored when jumping over C++ frames for Dart exceptions
in Exceptions::JumpToFrame().
fixes#31356
Change-Id: I71c2e86d1d4f24571dd618a5db06fd1277339ebc
Reviewed-on: https://dart-review.googlesource.com/23141
Commit-Queue: Zach Anderson <zra@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
This reverts commit af251ddfec.
Reason for revert: Crashes on Windows.
Original change's description:
> [vm] Prefer stack bounds from the system thread library for overflow checks.
>
> Before this change, we would assume all stacks have the size the VM specifies for threads it creates, but this won't be accurate for the initial thread or threads created by the embedder.
>
> Change-Id: I6605a88d6666a6f9d47fbe047d3fdd02aa22c80a
> Reviewed-on: https://dart-review.googlesource.com/10209
> Reviewed-by: Zach Anderson <zra@google.com>
TBR=rmacnak@google.com,zra@google.com
Change-Id: I96eaccdc8edf6e3d319827a63d168534bad0518e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/13106
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Before this change, we would assume all stacks have the size the VM specifies for threads it creates, but this won't be accurate for the initial thread or threads created by the embedder.
Change-Id: I6605a88d6666a6f9d47fbe047d3fdd02aa22c80a
Reviewed-on: https://dart-review.googlesource.com/10209
Reviewed-by: Zach Anderson <zra@google.com>
Like HOST_ARCH_*, HOST_OS_* describes the OS the VM is running on, which may be different from the OS the VM is generating code for during AOT compilation.
Currently we conflate the two when emitting AOT as assembly, and we get away with it because Flutter only uses assembly for targeting iOS and one can only target iOS from a Mac, but we expect to use assembly for Android as well so native tools can unwind Dart frames.
R=zra@google.com
Review-Url: https://codereview.chromium.org/2750843003 .
Currently it relies on OSThread::stack_base_ which is explicitly set.
However if we encounter a crash or assertion failure early we don't have stack_base_ set yet.
This CL allows us to get correct stack dump even if OSThread::stack_base_ is not set yet by falling back to low-level APIs:
* On Linux and Android we fallback to pthread_attr_getstack
* On Mac OS X we use pthread_get_stackaddr_np/pthread_get_stacksize_np.
* On Windows we read limits from TIB.
This code is mostly a port of the battle tested thread limits code from Oilpan GC inside Blink. The only differences is that we don't fallback to __libc_stack_end for the main thread.
We also cleanup GetAndValidateIsolateStackBounds a bit to avoid duplication there.
(This change is motivated by https://github.com/dart-lang/sdk/issues/28692 where crash in bootstrapping does not produce any useful stack trace)
BUG=
R=johnmccutchan@google.com
Review-Url: https://codereview.chromium.org/2680123004 .
We have a racy read of the object header in the stack frame walker
called from the profiler (in this case the gc marker is running at
the same time).
It is appropriate to avoid this even though we think the access is
safe as the objects are in old space and never forwarded and only
the mark bits are manipulated while marking
BUG=
R=johnmccutchan@google.com
Review URL: https://codereview.chromium.org/2517473002 .
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 #26400R=asiva@google.com, iposva@google.com
Review URL: https://codereview.chromium.org/1978153002 .
- [x] Refactor old OSThread interface for querying thread CPU time into OS::GetCurrentThreadCPUMicros.
- [x] Add data to TimelineEvent to track thread CPU time.
- [x] Include "tts" and "tdur" properties when converting the timeline to JSON
Fixes#26396
BUG=
R=rmacnak@google.com
Review URL: https://codereview.chromium.org/1960483002 .
in these blocks will not have a safepoint operation
- changed boxed_field_list_monitor_ to boxed_field_list_mutex_ as we only
need a mutex for guarding access to boxed_field_list_ as changed the
lock to use SafepointMutexLocker as the list addition code could potentially
allocate and result in GC (safepoint operation)
- Added a SafepointMonitorLocker as we have a function Isolate::VisitIsolates
which could potentially have safepoints in the enclosed block.
- Make the lock around MegamorphicCacheTable::Lookup a SafepointMutexLocker
as the look up code seems to be allocating memory while the lock is held.
- Changed the ThreadPool and MessageHandler code to account for the new
MonitorLocker usage standard
- Fixed PortMap::PrintPortsForMessageHandler to use SafepointMutexLocker as
the code it encloses calls into Dart
- Removed profiler_ field in class Profiler as it doesn't seem to be used.
R=johnmccutchan@google.com, srdjan@google.com
Review URL: https://codereview.chromium.org/1748953003 .
Vyacheslav Egorov