From 783d0ef90ac113d801dba312390fdfda0cf7d3c8 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Wed, 12 Apr 2023 16:39:19 +0000 Subject: [PATCH] Revert "[vm] Expose Dart_{CurrentIsolate,ExitIsolate,EnterIsolate}" This reverts commit a2512819f82890a1493c2454001c9e7b21a2c721. Reason for revert: FFI tests fail to link on Windows, fail to load on product-mode Android Original change's description: > [vm] Expose Dart_{CurrentIsolate,ExitIsolate,EnterIsolate} > > For applications that want to have arbitrary number of isolates call > into native code that may be blocking, we expose the API functions that > allows those native threads to exit an isolate before running > long/blocking code. > > Without the ability to exit/re-enter isolate, one may experience > deadlocks as we have a fixed limit on the number of concurrently > executing isolates atm. > > In the longer term we may find a way to do this automatically > with low overhead, see [0]. But since those API functions are quite > stable and we already expose e.g. `Dart_{Enter,Exit}Scope`, I don't > see a reason not to expose `Dart_{Enter,Exit}Isolate`. > > [0] Issue https://github.com/dart-lang/sdk/issues/51261 > > Issue https://github.com/dart-lang/sdk/issues/51254 > > TEST=ffi{,_2}/dl_api_exit_enter_isolate_test > > Change-Id: I91c772ca962fddb87919663fea07939a498fa205 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292722 > Commit-Queue: Martin Kustermann > Reviewed-by: Daco Harkes > Reviewed-by: Ryan Macnak Change-Id: I05ad5b9ce24754a68693160e470f8eb71a812c75 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/294860 Auto-Submit: Ryan Macnak Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper --- .../ffi_test/ffi_test_functions_vmspecific.cc | 34 ++++++------------ runtime/include/dart_api_dl.h | 4 --- runtime/include/dart_version.h | 2 +- tests/ffi/dl_api_exit_enter_isolate_test.dart | 33 ----------------- .../ffi_2/dl_api_exit_enter_isolate_test.dart | 35 ------------------- 5 files changed, 11 insertions(+), 97 deletions(-) delete mode 100644 tests/ffi/dl_api_exit_enter_isolate_test.dart delete mode 100644 tests/ffi_2/dl_api_exit_enter_isolate_test.dart diff --git a/runtime/bin/ffi_test/ffi_test_functions_vmspecific.cc b/runtime/bin/ffi_test/ffi_test_functions_vmspecific.cc index 97d5e71ac9fa..9ecaa7ec6880 100644 --- a/runtime/bin/ffi_test/ffi_test_functions_vmspecific.cc +++ b/runtime/bin/ffi_test/ffi_test_functions_vmspecific.cc @@ -30,8 +30,6 @@ #include #include -#include "bin/lockers.h" - // TODO(dartbug.com/40579): This requires static linking to either link // dart.exe or dart_precompiled_runtime.exe on Windows. // The sample currently fails on Windows in AOT mode. @@ -422,6 +420,16 @@ DART_EXPORT intptr_t InitDartApiDL(void* data) { // // sample_async_callback.dart +void Fatal(char const* file, int line, char const* error) { + printf("FATAL %s:%i\n", file, line); + printf("%s\n", error); + Dart_DumpNativeStackTrace(nullptr); + Dart_PrepareToAbort(); + abort(); +} + +#define FATAL(error) Fatal(__FILE__, __LINE__, error) + DART_EXPORT void SleepOnAnyOS(intptr_t seconds) { #if defined(DART_HOST_OS_WINDOWS) Sleep(1000 * seconds); @@ -1274,28 +1282,6 @@ DART_EXPORT void SetFfiNativeResolverForTest(Dart_Handle url) { ENSURE(!Dart_IsError(result)); } -DART_EXPORT void WaitUntilNThreadsEnterBarrier(intptr_t num_threads) { - Dart_Isolate isolate = Dart_CurrentIsolate_DL(); - Dart_ExitIsolate_DL(); - { - ENSURE(Dart_CurrentIsolate_DL() == nullptr); - - // Guaranteed to be initialized exactly once (no race between multiple - // threads). - static dart::bin::Monitor monitor; - static intptr_t thread_count = 0; - - dart::bin::MonitorLocker ml(&monitor); - ++thread_count; - while (thread_count < num_threads) { - ml.Wait(); - } - if (thread_count != num_threads) UNREACHABLE(); - ml.NotifyAll(); - } - Dart_EnterIsolate_DL(isolate); -} - //////////////////////////////////////////////////////////////////////////////// // Helper for the regression test for b/216834909 //////////////////////////////////////////////////////////////////////////////// diff --git a/runtime/include/dart_api_dl.h b/runtime/include/dart_api_dl.h index cce34500947a..ba50d992369a 100644 --- a/runtime/include/dart_api_dl.h +++ b/runtime/include/dart_api_dl.h @@ -90,10 +90,6 @@ typedef void (*Dart_NativeMessageHandler_DL)(Dart_Port_DL dest_port_id, F(Dart_UpdateFinalizableExternalSize, void, \ (Dart_FinalizableHandle object, Dart_Handle strong_ref_to_object, \ intptr_t external_allocation_size)) \ - /* Isolates */ \ - F(Dart_CurrentIsolate, Dart_Isolate, (void)) \ - F(Dart_ExitIsolate, void, (void)) \ - F(Dart_EnterIsolate, void, (Dart_Isolate)) \ /* Dart_Port */ \ F(Dart_Post, bool, (Dart_Port_DL port_id, Dart_Handle object)) \ F(Dart_NewSendPort, Dart_Handle, (Dart_Port_DL port_id)) \ diff --git a/runtime/include/dart_version.h b/runtime/include/dart_version.h index e2d3651fbd3a..680fb539bbea 100644 --- a/runtime/include/dart_version.h +++ b/runtime/include/dart_version.h @@ -11,6 +11,6 @@ // On backwards compatible changes the minor version is increased. // The versioning covers the symbols exposed in dart_api_dl.h #define DART_API_DL_MAJOR_VERSION 2 -#define DART_API_DL_MINOR_VERSION 3 +#define DART_API_DL_MINOR_VERSION 2 #endif /* RUNTIME_INCLUDE_DART_VERSION_H_ */ /* NOLINT */ diff --git a/tests/ffi/dl_api_exit_enter_isolate_test.dart b/tests/ffi/dl_api_exit_enter_isolate_test.dart deleted file mode 100644 index 5842cc8ba728..000000000000 --- a/tests/ffi/dl_api_exit_enter_isolate_test.dart +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) 2023, 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. - -// SharedObjects=ffi_test_functions - -import 'dart:ffi'; -import 'dart:isolate'; - -import 'package:expect/expect.dart'; - -import 'dylib_utils.dart'; - -final ffiTestFunctions = dlopenPlatformSpecific("ffi_test_functions"); - -final initializeApi = ffiTestFunctions.lookupFunction< - IntPtr Function(Pointer), - int Function(Pointer)>("InitDartApiDL"); -final enterBarrier = - ffiTestFunctions.lookupFunction( - "WaitUntilNThreadsEnterBarrier"); - -main() async { - const threadBarrierCount = 30; - - initializeApi(NativeApi.initializeApiDLData); - - final all = []; - for (int i = 0; i < threadBarrierCount; ++i) { - all.add(Isolate.run(() => enterBarrier(threadBarrierCount))); - } - await Future.wait(all); -} diff --git a/tests/ffi_2/dl_api_exit_enter_isolate_test.dart b/tests/ffi_2/dl_api_exit_enter_isolate_test.dart deleted file mode 100644 index 03abbb3ed50f..000000000000 --- a/tests/ffi_2/dl_api_exit_enter_isolate_test.dart +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright (c) 2023, 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. - -// @dart = 2.9 - -// SharedObjects=ffi_test_functions - -import 'dart:ffi'; -import 'dart:isolate'; - -import 'package:expect/expect.dart'; - -import 'dylib_utils.dart'; - -final ffiTestFunctions = dlopenPlatformSpecific("ffi_test_functions"); - -final initializeApi = ffiTestFunctions.lookupFunction< - IntPtr Function(Pointer), - int Function(Pointer)>("InitDartApiDL"); -final enterBarrier = - ffiTestFunctions.lookupFunction( - "WaitUntilNThreadsEnterBarrier"); - -main() async { - const threadBarrierCount = 30; - - initializeApi(NativeApi.initializeApiDLData); - - final all = []; - for (int i = 0; i < threadBarrierCount; ++i) { - all.add(Isolate.run(() => enterBarrier(threadBarrierCount))); - } - await Future.wait(all); -}