From 42b70dcddbc2d219685de3d6fe459efae88f1915 Mon Sep 17 00:00:00 2001 From: "sgjesse@google.com" Date: Thu, 2 Feb 2012 08:59:26 +0000 Subject: [PATCH] Change the thread interface in runtime/platform and use it starting all threads The platform thread interface (dart::thread) is now refactored to an all static interface as suggested by iposva@ and asiva@. Use this interface for running all threads in the VM. R=ager@google.com, iposva@google.com BUG= TEST= Review URL: https://chromiumcodereview.appspot.com//9141005 git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@3830 260f80e4-7a28-3924-810f-c04153c831b5 --- runtime/bin/builtin_impl_sources.gypi | 6 ---- runtime/bin/eventhandler_linux.cc | 16 ++++----- runtime/bin/eventhandler_linux.h | 2 +- runtime/bin/eventhandler_macos.cc | 16 ++++----- runtime/bin/eventhandler_macos.h | 2 +- runtime/bin/eventhandler_win.cc | 17 ++++----- runtime/bin/process_linux.cc | 5 ++- runtime/bin/process_macos.cc | 5 ++- runtime/bin/thread_pool.cc | 35 ++++++++++++++++-- runtime/bin/thread_pool.h | 18 +++------- runtime/bin/thread_pool_linux.cc | 33 ----------------- runtime/bin/thread_pool_linux.h | 33 ----------------- runtime/bin/thread_pool_macos.cc | 33 ----------------- runtime/bin/thread_pool_macos.h | 33 ----------------- runtime/bin/thread_pool_win.cc | 14 -------- runtime/bin/thread_pool_win.h | 27 -------------- runtime/lib/isolate.cc | 5 ++- runtime/platform/thread.h | 13 +++---- runtime/platform/thread_linux.cc | 49 +++++++++++++------------ runtime/platform/thread_linux.h | 16 --------- runtime/platform/thread_macos.cc | 51 +++++++++++++-------------- runtime/platform/thread_macos.h | 16 --------- runtime/platform/thread_win.cc | 34 ++++++++---------- runtime/platform/thread_win.h | 15 -------- runtime/vm/dart_api_impl_test.cc | 4 +-- runtime/vm/message_test.cc | 4 +-- runtime/vm/native_message_handler.cc | 5 ++- runtime/vm/port_test.cc | 4 +-- 28 files changed, 150 insertions(+), 361 deletions(-) delete mode 100644 runtime/bin/thread_pool_linux.cc delete mode 100644 runtime/bin/thread_pool_linux.h delete mode 100644 runtime/bin/thread_pool_macos.cc delete mode 100644 runtime/bin/thread_pool_macos.h delete mode 100644 runtime/bin/thread_pool_win.cc delete mode 100644 runtime/bin/thread_pool_win.h diff --git a/runtime/bin/builtin_impl_sources.gypi b/runtime/bin/builtin_impl_sources.gypi index 523d1b61459..c5c1cea6b26 100644 --- a/runtime/bin/builtin_impl_sources.gypi +++ b/runtime/bin/builtin_impl_sources.gypi @@ -51,12 +51,6 @@ 'set_test.cc', 'thread.h', 'thread_pool.cc', - 'thread_pool_linux.cc', - 'thread_pool_linux.h', - 'thread_pool_macos.cc', - 'thread_pool_macos.h', 'thread_pool_test.cc', - 'thread_pool_win.cc', - 'thread_pool_win.h', ], } diff --git a/runtime/bin/eventhandler_linux.cc b/runtime/bin/eventhandler_linux.cc index e98ffa042e3..c46c89c0b06 100644 --- a/runtime/bin/eventhandler_linux.cc +++ b/runtime/bin/eventhandler_linux.cc @@ -2,6 +2,8 @@ // 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. +#include "bin/eventhandler.h" + #include #include #include @@ -10,7 +12,6 @@ #include #include -#include "bin/eventhandler.h" #include "bin/fdutils.h" #include "bin/hashmap.h" #include "platform/utils.h" @@ -339,11 +340,12 @@ void EventHandlerImplementation::HandleTimeout() { } -void* EventHandlerImplementation::Poll(void* args) { +void EventHandlerImplementation::Poll(uword args) { intptr_t pollfds_size; struct pollfd* pollfds; EventHandlerImplementation* handler = reinterpret_cast(args); + ASSERT(handler != NULL); while (1) { pollfds = handler->GetPollFds(&pollfds_size); intptr_t millis = handler->GetTimeout(); @@ -359,18 +361,14 @@ void* EventHandlerImplementation::Poll(void* args) { } free(pollfds); } - return NULL; } void EventHandlerImplementation::StartEventHandler() { - pthread_t handler_thread; - int result = pthread_create(&handler_thread, - NULL, - &EventHandlerImplementation::Poll, - this); + int result = dart::Thread::Start(&EventHandlerImplementation::Poll, + reinterpret_cast(this)); if (result != 0) { - FATAL("Create start event handler thread"); + FATAL1("Failed to start event handler thread %d", result); } } diff --git a/runtime/bin/eventhandler_linux.h b/runtime/bin/eventhandler_linux.h index 217bdb95529..b210e774ca8 100644 --- a/runtime/bin/eventhandler_linux.h +++ b/runtime/bin/eventhandler_linux.h @@ -103,7 +103,7 @@ class EventHandlerImplementation { struct pollfd* GetPollFds(intptr_t* size); void HandleEvents(struct pollfd* pollfds, int pollfds_size, int result_size); void HandleTimeout(); - static void* Poll(void* args); + static void Poll(uword args); void WakeupHandler(intptr_t id, Dart_Port dart_port, int64_t data); void HandleInterruptFd(); void SetPort(intptr_t fd, Dart_Port dart_port, intptr_t mask); diff --git a/runtime/bin/eventhandler_macos.cc b/runtime/bin/eventhandler_macos.cc index d4aa0841d42..188720ce776 100644 --- a/runtime/bin/eventhandler_macos.cc +++ b/runtime/bin/eventhandler_macos.cc @@ -2,6 +2,8 @@ // 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. +#include "bin/eventhandler.h" + #include #include #include @@ -10,7 +12,6 @@ #include #include -#include "bin/eventhandler.h" #include "bin/fdutils.h" #include "bin/hashmap.h" #include "platform/utils.h" @@ -337,11 +338,12 @@ void EventHandlerImplementation::HandleTimeout() { } -void* EventHandlerImplementation::Poll(void* args) { +void EventHandlerImplementation::Poll(uword args) { intptr_t pollfds_size; struct pollfd* pollfds; EventHandlerImplementation* handler = reinterpret_cast(args); + ASSERT(handler != NULL); while (1) { pollfds = handler->GetPollFds(&pollfds_size); intptr_t millis = handler->GetTimeout(); @@ -357,18 +359,14 @@ void* EventHandlerImplementation::Poll(void* args) { } free(pollfds); } - return NULL; } void EventHandlerImplementation::StartEventHandler() { - pthread_t handler_thread; - int result = pthread_create(&handler_thread, - NULL, - &EventHandlerImplementation::Poll, - this); + int result = dart::Thread::Start(&EventHandlerImplementation::Poll, + reinterpret_cast(this)); if (result != 0) { - FATAL("Create start event handler thread"); + FATAL1("Failed to start event handler thread %d", result); } } diff --git a/runtime/bin/eventhandler_macos.h b/runtime/bin/eventhandler_macos.h index 014f27febfc..9ee388b99a7 100644 --- a/runtime/bin/eventhandler_macos.h +++ b/runtime/bin/eventhandler_macos.h @@ -103,7 +103,7 @@ class EventHandlerImplementation { struct pollfd* GetPollFds(intptr_t* size); void HandleEvents(struct pollfd* pollfds, int pollfds_size, int result_size); void HandleTimeout(); - static void* Poll(void* args); + static void Poll(uword args); void WakeupHandler(intptr_t id, Dart_Port dart_port, int64_t data); void HandleInterruptFd(); void SetPort(intptr_t fd, Dart_Port dart_port, intptr_t mask); diff --git a/runtime/bin/eventhandler_win.cc b/runtime/bin/eventhandler_win.cc index 043db3c7a1d..72311c109ed 100644 --- a/runtime/bin/eventhandler_win.cc +++ b/runtime/bin/eventhandler_win.cc @@ -1,14 +1,15 @@ -// Copyright (c) 2011, the Dart project authors. Please see the AUTHORS file +// Copyright (c) 2012, 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. +#include "bin/eventhandler.h" + #include #include #include #include #include "bin/builtin.h" -#include "bin/eventhandler.h" #include "bin/socket.h" @@ -823,9 +824,10 @@ void EventHandlerImplementation::SendData(intptr_t id, } -static unsigned int __stdcall EventHandlerThread(void* args) { +static void EventHandlerThread(uword args) { EventHandlerImplementation* handler = reinterpret_cast(args); + ASSERT(handler != NULL); while (true) { DWORD bytes; ULONG_PTR key; @@ -877,11 +879,10 @@ static unsigned int __stdcall EventHandlerThread(void* args) { void EventHandlerImplementation::StartEventHandler() { - uint32_t tid; - uintptr_t thread_handle = - _beginthreadex(NULL, 32 * 1024, EventHandlerThread, this, 0, &tid); - if (thread_handle == -1) { - FATAL("Failed to start event handler thread"); + int result = dart::Thread::Start(EventHandlerThread, + reinterpret_cast(this)); + if (result != 0) { + FATAL1("Failed to start event handler thread %d", result); } // Initialize Winsock32 diff --git a/runtime/bin/process_linux.cc b/runtime/bin/process_linux.cc index 3684f764d44..c2c86f19dae 100644 --- a/runtime/bin/process_linux.cc +++ b/runtime/bin/process_linux.cc @@ -120,7 +120,10 @@ class ExitCodeHandler { // Start thread that polls the pipe and handles process exits when // data is received on the pipe. - new dart::Thread(ExitCodeHandlerEntry, sig_chld_fds_[0]); + result = dart::Thread::Start(ExitCodeHandlerEntry, sig_chld_fds_[0]); + if (result != 0) { + FATAL1("Failed to start exit code handler worker thread %d", result); + } // Mark write end non-blocking. FDUtils::SetNonBlocking(sig_chld_fds_[1]); diff --git a/runtime/bin/process_macos.cc b/runtime/bin/process_macos.cc index b5ac36c08b5..13221c092d5 100644 --- a/runtime/bin/process_macos.cc +++ b/runtime/bin/process_macos.cc @@ -119,7 +119,10 @@ class ExitCodeHandler { // Start thread that polls the pipe and handles process exits when // data is received on the pipe. - new dart::Thread(ExitCodeHandlerEntry, sig_chld_fds_[0]); + result = dart::Thread::Start(ExitCodeHandlerEntry, sig_chld_fds_[0]); + if (result != 0) { + FATAL1("Failed to start exit code handler worker thread %d", result); + } // Mark write end non-blocking. FDUtils::SetNonBlocking(sig_chld_fds_[1]); diff --git a/runtime/bin/thread_pool.cc b/runtime/bin/thread_pool.cc index 36960a72303..cf1ec3b9d4c 100644 --- a/runtime/bin/thread_pool.cc +++ b/runtime/bin/thread_pool.cc @@ -63,7 +63,36 @@ Task ThreadPool::WaitForTask() { } -void* ThreadPool::Main(void* args) { +void ThreadPool::Start() { + MonitorLocker monitor(&monitor_); + for (int i = 0; i < size_; i++) { + int result = dart::Thread::Start(&ThreadPool::Main, + reinterpret_cast(this)); + if (result != 0) { + FATAL1("Failed to start thread pool thread %d", result); + } + } +} + + +void ThreadPool::Shutdown() { + terminate_ = true; + queue_.Shutdown(); + MonitorLocker monitor(&monitor_); + while (size_ > 0) { + monitor.Wait(); + } +} + + +void ThreadPool::ThreadTerminated() { + MonitorLocker monitor(&monitor_); + size_--; + monitor.Notify(); +} + + +void ThreadPool::Main(uword args) { if (Dart_IsVMFlagSet("trace_thread_pool")) { printf("Thread pool thread started\n"); } @@ -73,8 +102,8 @@ void* ThreadPool::Main(void* args) { printf("Waiting for task\n"); } Task task = pool->WaitForTask(); - if (pool->terminate_) return NULL; + if (pool->terminate_) break; (*(pool->task_handler_))(task); } - return NULL; + pool->ThreadTerminated(); }; diff --git a/runtime/bin/thread_pool.h b/runtime/bin/thread_pool.h index 10cbba44de0..3eaa2e4901e 100644 --- a/runtime/bin/thread_pool.h +++ b/runtime/bin/thread_pool.h @@ -9,18 +9,6 @@ #include "platform/globals.h" #include "platform/thread.h" -// Declare the OS-specific types ahead of defining the generic classes. -#if defined(TARGET_OS_LINUX) -#include "bin/thread_pool_linux.h" -#elif defined(TARGET_OS_MACOS) -#include "bin/thread_pool_macos.h" -#elif defined(TARGET_OS_WINDOWS) -#include "bin/thread_pool_win.h" -#else -#error Unknown target os. -#endif - - typedef void* Task; @@ -74,18 +62,20 @@ class ThreadPool { void InsertTask(Task task); + void ThreadTerminated(); + private: Task WaitForTask(); - static void* Main(void* args); + static void Main(uword args); TaskQueue queue_; // TODO(sgjesse): Move the monitor in TaskQueue to ThreadPool and // obtain it for updating terminate_. + dart::Monitor monitor_; bool terminate_; int size_; // Number of threads. TaskHandler task_handler_; - ThreadPoolData data_; DISALLOW_COPY_AND_ASSIGN(ThreadPool); }; diff --git a/runtime/bin/thread_pool_linux.cc b/runtime/bin/thread_pool_linux.cc deleted file mode 100644 index ce6c10f2de9..00000000000 --- a/runtime/bin/thread_pool_linux.cc +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) 2012, 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. - -#include - -#include "bin/thread_pool.h" - -void ThreadPool::Start() { - pthread_t* threads - = reinterpret_cast(calloc(size_, sizeof(pthread_t*))); // NOLINT - data_.set_threads(threads); - for (int i = 0; i < size_; i++) { - pthread_t handler_thread; - int result = pthread_create(&handler_thread, - NULL, - &ThreadPool::Main, - this); - if (result != 0) { - FATAL("Create and start thread pool thread"); - } - data_.threads()[i] = handler_thread; - } -} - - -void ThreadPool::Shutdown() { - terminate_ = true; - queue_.Shutdown(); - for (int i = 0; i < size_; i++) { - pthread_join(data_.threads()[i], NULL); - } -} diff --git a/runtime/bin/thread_pool_linux.h b/runtime/bin/thread_pool_linux.h deleted file mode 100644 index 84211975803..00000000000 --- a/runtime/bin/thread_pool_linux.h +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) 2012, 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. - -#ifndef BIN_THREAD_POOL_LINUX_H_ -#define BIN_THREAD_POOL_LINUX_H_ - -#if !defined(BIN_THREAD_POOL_H_) -#error Do not include thread_pool_linux.h directly; use thread_pool.h instead. -#endif - -#include - -#include "platform/globals.h" - - -class ThreadPoolData { - private: - ThreadPoolData() {} - ~ThreadPoolData() {} - - pthread_t* threads() { return threads_; } - void set_threads(pthread_t* threads) { threads_ = threads; } - - pthread_t* threads_; - - friend class ThreadPool; - - DISALLOW_ALLOCATION(); - DISALLOW_COPY_AND_ASSIGN(ThreadPoolData); -}; - -#endif // BIN_THREAD_POOL_LINUX_H_ diff --git a/runtime/bin/thread_pool_macos.cc b/runtime/bin/thread_pool_macos.cc deleted file mode 100644 index ce6c10f2de9..00000000000 --- a/runtime/bin/thread_pool_macos.cc +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) 2012, 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. - -#include - -#include "bin/thread_pool.h" - -void ThreadPool::Start() { - pthread_t* threads - = reinterpret_cast(calloc(size_, sizeof(pthread_t*))); // NOLINT - data_.set_threads(threads); - for (int i = 0; i < size_; i++) { - pthread_t handler_thread; - int result = pthread_create(&handler_thread, - NULL, - &ThreadPool::Main, - this); - if (result != 0) { - FATAL("Create and start thread pool thread"); - } - data_.threads()[i] = handler_thread; - } -} - - -void ThreadPool::Shutdown() { - terminate_ = true; - queue_.Shutdown(); - for (int i = 0; i < size_; i++) { - pthread_join(data_.threads()[i], NULL); - } -} diff --git a/runtime/bin/thread_pool_macos.h b/runtime/bin/thread_pool_macos.h deleted file mode 100644 index b24a73e3283..00000000000 --- a/runtime/bin/thread_pool_macos.h +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) 2012, 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. - -#ifndef BIN_THREAD_POOL_MACOS_H_ -#define BIN_THREAD_POOL_MACOS_H_ - -#if !defined(BIN_THREAD_POOL_H_) -#error Do not include thread_pool_macos.h directly; use thread_pool.h instead. -#endif - -#include - -#include "platform/globals.h" - - -class ThreadPoolData { - private: - ThreadPoolData() {} - ~ThreadPoolData() {} - - pthread_t* threads() { return threads_; } - void set_threads(pthread_t* threads) { threads_ = threads; } - - pthread_t* threads_; - - friend class ThreadPool; - - DISALLOW_ALLOCATION(); - DISALLOW_COPY_AND_ASSIGN(ThreadPoolData); -}; - -#endif // BIN_THREAD_POOL_MACOS_H_ diff --git a/runtime/bin/thread_pool_win.cc b/runtime/bin/thread_pool_win.cc deleted file mode 100644 index cf6b860e76b..00000000000 --- a/runtime/bin/thread_pool_win.cc +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) 2012, 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. - -#include "bin/thread_pool.h" - -void ThreadPool::Start() { - UNIMPLEMENTED(); -} - - -void ThreadPool::Shutdown() { - UNIMPLEMENTED(); -} diff --git a/runtime/bin/thread_pool_win.h b/runtime/bin/thread_pool_win.h deleted file mode 100644 index 5a6f450aca2..00000000000 --- a/runtime/bin/thread_pool_win.h +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright (c) 2012, 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. - -#ifndef BIN_THREAD_POOL_WIN_H_ -#define BIN_THREAD_POOL_WIN_H_ - -#if !defined(BIN_THREAD_POOL_H_) -#error Do not include thread_pool_win.h directly; use thread_pool.h instead. -#endif - -#include "platform/globals.h" - -class ThreadPoolData { - private: - static const int kMaxThreadPoolSize = 16; - - ThreadPoolData() { UNIMPLEMENTED(); } - ~ThreadPoolData() {} - - friend class ThreadPool; - - DISALLOW_ALLOCATION(); - DISALLOW_COPY_AND_ASSIGN(ThreadPoolData); -}; - -#endif // BIN_THREAD_POOL_WIN_H_ diff --git a/runtime/lib/isolate.cc b/runtime/lib/isolate.cc index decfaa04e74..5760df1bf2c 100644 --- a/runtime/lib/isolate.cc +++ b/runtime/lib/isolate.cc @@ -299,7 +299,10 @@ DEFINE_NATIVE_ENTRY(IsolateNatives_start, 2) { strdup(library_url), strdup(class_name), port_id)); - new Thread(RunIsolate, data); + int result = Thread::Start(RunIsolate, data); + if (result != 0) { + FATAL1("Failed to start isolate thread %d", result); + } } else { // Error spawning the isolate, maybe due to initialization errors or // errors while loading the application into spawned isolate, shut diff --git a/runtime/platform/thread.h b/runtime/platform/thread.h index 0eb8da13b2b..603949e9ca7 100644 --- a/runtime/platform/thread.h +++ b/runtime/platform/thread.h @@ -22,17 +22,12 @@ namespace dart { class Thread { public: - // Function to be called on thread start. typedef void (*ThreadStartFunction) (uword parameter); - // TODO(iposva): Define the proper interface for spawning and killing threads. - Thread(ThreadStartFunction function, uword parameters); - ~Thread(); - - private: - ThreadData data_; - - DISALLOW_COPY_AND_ASSIGN(Thread); + // Start a thread running the specified function. Returns 0 if the + // thread started successfuly and a system specific error code if + // the thread failed to start. + static int Start(ThreadStartFunction function, uword parameters); }; diff --git a/runtime/platform/thread_linux.cc b/runtime/platform/thread_linux.cc index 338c4c8d1ca..7efa2b5d09b 100644 --- a/runtime/platform/thread_linux.cc +++ b/runtime/platform/thread_linux.cc @@ -17,6 +17,19 @@ namespace dart { } +#ifdef DEBUG +#define RETURN_ON_PTHREAD_FAILURE(result) \ + if (result != 0) { \ + fprintf(stderr, "%s:%d: pthread error: %d (%s)\n", \ + __FILE__, __LINE__, result, strerror(result)); \ + return result; \ + } +#else +#define RETURN_ON_PTHREAD_FAILURE(result) \ + if (result != 0) return result; +#endif + + static void ComputeTimeSpec(struct timespec* ts, int64_t millis) { int64_t secs = millis / kMillisecondsPerSecond; int64_t nanos = @@ -35,18 +48,15 @@ static void ComputeTimeSpec(struct timespec* ts, int64_t millis) { class ThreadStartData { public: ThreadStartData(Thread::ThreadStartFunction function, - uword parameter, - Thread* thread) - : function_(function), parameter_(parameter), thread_(thread) {} + uword parameter) + : function_(function), parameter_(parameter) {} Thread::ThreadStartFunction function() const { return function_; } uword parameter() const { return parameter_; } - Thread* thread() const { return thread_; } private: Thread::ThreadStartFunction function_; uword parameter_; - Thread* thread_; DISALLOW_COPY_AND_ASSIGN(ThreadStartData); }; @@ -60,47 +70,36 @@ static void* ThreadStart(void* data_ptr) { Thread::ThreadStartFunction function = data->function(); uword parameter = data->parameter(); - Thread* thread = data->thread(); delete data; // Call the supplied thread start function handing it its parameters. function(parameter); - // When the function returns here, make sure that the thread is deleted. - delete thread; - return NULL; } -Thread::Thread(ThreadStartFunction function, uword parameter) { +int Thread::Start(ThreadStartFunction function, uword parameter) { pthread_attr_t attr; int result = pthread_attr_init(&attr); - VALIDATE_PTHREAD_RESULT(result); + RETURN_ON_PTHREAD_FAILURE(result); result = pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); - VALIDATE_PTHREAD_RESULT(result); + RETURN_ON_PTHREAD_FAILURE(result); result = pthread_attr_setstacksize(&attr, 1024 * KB); - VALIDATE_PTHREAD_RESULT(result); + RETURN_ON_PTHREAD_FAILURE(result); - ThreadStartData* data = new ThreadStartData(function, parameter, this); + ThreadStartData* data = new ThreadStartData(function, parameter); pthread_t tid; - result = pthread_create(&tid, - &attr, - ThreadStart, - data); - VALIDATE_PTHREAD_RESULT(result); - - data_.tid_ = tid; + result = pthread_create(&tid, &attr, ThreadStart, data); + RETURN_ON_PTHREAD_FAILURE(result); result = pthread_attr_destroy(&attr); - VALIDATE_PTHREAD_RESULT(result); -} + RETURN_ON_PTHREAD_FAILURE(result); - -Thread::~Thread() { + return 0; } diff --git a/runtime/platform/thread_linux.h b/runtime/platform/thread_linux.h index 7e15e5e2838..a32c680b6d5 100644 --- a/runtime/platform/thread_linux.h +++ b/runtime/platform/thread_linux.h @@ -16,22 +16,6 @@ namespace dart { -class ThreadData { - private: - ThreadData() {} - ~ThreadData() {} - - pthread_t* tid() { return &tid_; } - - pthread_t tid_; - - friend class Thread; - - DISALLOW_ALLOCATION(); - DISALLOW_COPY_AND_ASSIGN(ThreadData); -}; - - class MutexData { private: MutexData() {} diff --git a/runtime/platform/thread_macos.cc b/runtime/platform/thread_macos.cc index 29e57191579..6fa80ce738f 100644 --- a/runtime/platform/thread_macos.cc +++ b/runtime/platform/thread_macos.cc @@ -10,27 +10,37 @@ namespace dart { -#define VALIDATE_PTHREAD_RESULT(result) \ +#define VALIDATE_PTHREAD_RESULT(result) \ if (result != 0) { \ FATAL2("pthread error: %d (%s)", result, strerror(result)); \ } +#ifdef DEBUG +#define RETURN_ON_PTHREAD_FAILURE(result) \ + if (result != 0) { \ + fprintf(stderr, "%s:%d: pthread error: %d (%s)\n", \ + __FILE__, __LINE__, result, strerror(result)); \ + return result; \ + } +#else +#define RETURN_ON_PTHREAD_FAILURE(result) \ + if (result != 0) return result; +#endif + + class ThreadStartData { public: ThreadStartData(Thread::ThreadStartFunction function, - uword parameter, - Thread* thread) - : function_(function), parameter_(parameter), thread_(thread) {} + uword parameter) + : function_(function), parameter_(parameter) {} Thread::ThreadStartFunction function() const { return function_; } uword parameter() const { return parameter_; } - Thread* thread() const { return thread_; } private: Thread::ThreadStartFunction function_; uword parameter_; - Thread* thread_; DISALLOW_COPY_AND_ASSIGN(ThreadStartData); }; @@ -44,47 +54,36 @@ static void* ThreadStart(void* data_ptr) { Thread::ThreadStartFunction function = data->function(); uword parameter = data->parameter(); - Thread* thread = data->thread(); delete data; // Call the supplied thread start function handing it its parameters. function(parameter); - // When the function returns here, make sure that the thread is deleted. - delete thread; - return NULL; } -Thread::Thread(ThreadStartFunction function, uword parameter) { +int Thread::Start(ThreadStartFunction function, uword parameter) { pthread_attr_t attr; int result = pthread_attr_init(&attr); - VALIDATE_PTHREAD_RESULT(result); + RETURN_ON_PTHREAD_FAILURE(result); result = pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); - VALIDATE_PTHREAD_RESULT(result); + RETURN_ON_PTHREAD_FAILURE(result); result = pthread_attr_setstacksize(&attr, 128 * KB); - VALIDATE_PTHREAD_RESULT(result); + RETURN_ON_PTHREAD_FAILURE(result); - ThreadStartData* data = new ThreadStartData(function, parameter, this); + ThreadStartData* data = new ThreadStartData(function, parameter); pthread_t tid; - result = pthread_create(&tid, - &attr, - ThreadStart, - data); - VALIDATE_PTHREAD_RESULT(result); - - data_.tid_ = tid; + result = pthread_create(&tid, &attr, ThreadStart, data); + RETURN_ON_PTHREAD_FAILURE(result); result = pthread_attr_destroy(&attr); - VALIDATE_PTHREAD_RESULT(result); -} + RETURN_ON_PTHREAD_FAILURE(result); - -Thread::~Thread() { + return 0; } diff --git a/runtime/platform/thread_macos.h b/runtime/platform/thread_macos.h index a836dc31aa4..c2b30595c15 100644 --- a/runtime/platform/thread_macos.h +++ b/runtime/platform/thread_macos.h @@ -15,22 +15,6 @@ namespace dart { -class ThreadData { - private: - ThreadData() {} - ~ThreadData() {} - - pthread_t* tid() { return &tid_; } - - pthread_t tid_; - - friend class Thread; - - DISALLOW_ALLOCATION(); - DISALLOW_COPY_AND_ASSIGN(ThreadData); -}; - - class MutexData { private: MutexData() {} diff --git a/runtime/platform/thread_win.cc b/runtime/platform/thread_win.cc index e3d76ea0164..17d3c59867a 100644 --- a/runtime/platform/thread_win.cc +++ b/runtime/platform/thread_win.cc @@ -12,19 +12,15 @@ namespace dart { class ThreadStartData { public: - ThreadStartData(Thread::ThreadStartFunction function, - uword parameter, - Thread* thread) - : function_(function), parameter_(parameter), thread_(thread) {} + ThreadStartData(Thread::ThreadStartFunction function, uword parameter) + : function_(function), parameter_(parameter) {} Thread::ThreadStartFunction function() const { return function_; } uword parameter() const { return parameter_; } - Thread* thread() const { return thread_; } private: Thread::ThreadStartFunction function_; uword parameter_; - Thread* thread_; DISALLOW_COPY_AND_ASSIGN(ThreadStartData); }; @@ -38,33 +34,31 @@ static unsigned int __stdcall ThreadEntry(void* data_ptr) { Thread::ThreadStartFunction function = data->function(); uword parameter = data->parameter(); - Thread* thread = data->thread(); delete data; // Call the supplied thread start function handing it its parameters. function(parameter); - // When the function returns here, make sure that the thread is deleted. - delete thread; + // When the function returns here close the handle. + CloseHandle(GetCurrentThread()); return 0; } -Thread::Thread(ThreadStartFunction function, uword parameter) { - ThreadStartData* start_data = new ThreadStartData(function, parameter, this); +int Thread::Start(ThreadStartFunction function, uword parameter) { + ThreadStartData* start_data = new ThreadStartData(function, parameter); uint32_t tid; - data_.thread_handle_ = - _beginthreadex(NULL, 64 * KB, ThreadEntry, start_data, 0, &tid); - if (data_.thread_handle_ == -1) { - FATAL("Thread creation failed"); + uintptr_t thread = + _beginthreadex(NULL, 64 * KB, ThreadEntry, start_data, 0, &tid); + if (thread == -1L || thread == 0) { +#ifdef DEBUG + fprintf(stderr, "_beginthreadex error: %d (%s)\n", errno, strerror(errno)); +#endif + return errno; } - data_.tid_ = tid; -} - -Thread::~Thread() { - CloseHandle(reinterpret_cast(data_.thread_handle_)); + return 0; } diff --git a/runtime/platform/thread_win.h b/runtime/platform/thread_win.h index 45235f19143..daa75e4d8cc 100644 --- a/runtime/platform/thread_win.h +++ b/runtime/platform/thread_win.h @@ -13,21 +13,6 @@ namespace dart { -class ThreadData { - private: - ThreadData() {} - ~ThreadData() {} - - uintptr_t thread_handle_; - uint32_t tid_; - - friend class Thread; - - DISALLOW_ALLOCATION(); - DISALLOW_COPY_AND_ASSIGN(ThreadData); -}; - - class MutexData { private: MutexData() {} diff --git a/runtime/vm/dart_api_impl_test.cc b/runtime/vm/dart_api_impl_test.cc index 4201d0de10e..f42dd48a262 100644 --- a/runtime/vm/dart_api_impl_test.cc +++ b/runtime/vm/dart_api_impl_test.cc @@ -3116,8 +3116,8 @@ TEST_CASE(IsolateInterrupt) { Isolate::SetInterruptCallback(IsolateInterruptTestCallback); sync = new Monitor(); - Thread* thread = new Thread(BusyLoop_start, 0); - EXPECT(thread != NULL); + int result = Thread::Start(BusyLoop_start, 0); + EXPECT_EQ(0, result); { MonitorLocker ml(sync); diff --git a/runtime/vm/message_test.cc b/runtime/vm/message_test.cc index 12ecac25a5c..dfbe1b93c42 100644 --- a/runtime/vm/message_test.cc +++ b/runtime/vm/message_test.cc @@ -155,8 +155,8 @@ void MessageReceiver_start(uword unused) { TEST_CASE(MessageQueue_WaitNotify) { sync = new Monitor(); - Thread* thread = new Thread(MessageReceiver_start, 0); - EXPECT(thread != NULL); + int result = Thread::Start(MessageReceiver_start, 0); + EXPECT_EQ(0, result); // Wait for the shared queue to be created. while (shared_queue == NULL) { diff --git a/runtime/vm/native_message_handler.cc b/runtime/vm/native_message_handler.cc index 5961acb9d75..ebc7121c848 100644 --- a/runtime/vm/native_message_handler.cc +++ b/runtime/vm/native_message_handler.cc @@ -60,7 +60,10 @@ static void RunWorker(uword parameter) { void NativeMessageHandler::StartWorker() { - new Thread(RunWorker, reinterpret_cast(this)); + int result = Thread::Start(RunWorker, reinterpret_cast(this)); + if (result != 0) { + FATAL1("Failed to start native message handler worker thread %d", result); + } } diff --git a/runtime/vm/port_test.cc b/runtime/vm/port_test.cc index 284c32fa149..b855cdb2ea6 100644 --- a/runtime/vm/port_test.cc +++ b/runtime/vm/port_test.cc @@ -193,8 +193,8 @@ void ThreadedPort_start(uword parameter) { TEST_CASE(ThreadedPort) { intptr_t local = PortMap::CreatePort(Isolate::Current()->message_handler()); - Thread* thr = new Thread(ThreadedPort_start, local); - EXPECT(thr != NULL); + int result = Thread::Start(ThreadedPort_start, local); + EXPECT_EQ(0, result); Message* msg = NextMessage(); EXPECT_EQ(local, msg->dest_port());