From 7c2585fba5639e11e3c939cecb3e39e788f75cb0 Mon Sep 17 00:00:00 2001 From: Simon Evans Date: Sat, 21 Oct 2017 15:02:35 +0100 Subject: [PATCH 1/3] Thread: Fixes to thread name - _CFThreadSetName(): Take the thread ID, as Linux allows setting the name of another thread. On macOS just check that it is the same as pthread_self(). - On Linux, trim the name to 15 characters as that is the maximum that can be set. - When starting a thread, set it's name in pthreads if the name is non-nil. - On Linux, make _thread an Optional initialised to nil instead of setting it to pthread_t(), which gives it the value 0. This matches macOS behaviour. - TestThread.swift: Add some more tests. --- CoreFoundation/Base.subproj/CFPlatform.c | 15 +++-- .../Base.subproj/ForSwiftFoundationOnly.h | 4 +- Foundation/Thread.swift | 14 ++-- TestFoundation/TestThread.swift | 65 ++++++++++++++----- 4 files changed, 72 insertions(+), 26 deletions(-) diff --git a/CoreFoundation/Base.subproj/CFPlatform.c b/CoreFoundation/Base.subproj/CFPlatform.c index 70597d1e7a..4e7e0f692f 100644 --- a/CoreFoundation/Base.subproj/CFPlatform.c +++ b/CoreFoundation/Base.subproj/CFPlatform.c @@ -1316,15 +1316,22 @@ _CFThreadRef _CFThreadCreate(const _CFThreadAttributes attrs, void *_Nullable (* return thread; } -CF_SWIFT_EXPORT void _CFThreadSetName(const char *_Nullable name) { +CF_SWIFT_EXPORT int _CFThreadSetName(pthread_t thread, const char *_Nonnull name) { #if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED || DEPLOYMENT_TARGET_EMBEDDED_MINI - pthread_setname_np(name); + if (pthread_equal(pthread_self(), thread)) { + return pthread_setname_np(name); + } + return EINVAL; #elif DEPLOYMENT_TARGET_LINUX - pthread_setname_np(pthread_self(), name); + // pthread_setname_np will fail if name >= 16 characters + char short_name[16]; + strncpy(short_name, name, 15); + short_name[15] = '\0'; + return pthread_setname_np(thread, short_name); #endif } -CF_SWIFT_EXPORT int _CFThreadGetName(char *buf, int length) { +CF_SWIFT_EXPORT int _CFThreadGetName(char *_Nonnull buf, int length) { #if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED || DEPLOYMENT_TARGET_EMBEDDED_MINI return pthread_getname_np(pthread_self(), buf, length); #elif DEPLOYMENT_TARGET_ANDROID diff --git a/CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h b/CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h index 2651cd89f5..1d0efbc99c 100644 --- a/CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h +++ b/CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h @@ -316,8 +316,8 @@ typedef pthread_t _CFThreadRef; CF_EXPORT _CFThreadRef _CFThreadCreate(const _CFThreadAttributes attrs, void *_Nullable (* _Nonnull startfn)(void *_Nullable), void *restrict _Nullable context); -CF_SWIFT_EXPORT void _CFThreadSetName(const char *_Nullable name); -CF_SWIFT_EXPORT int _CFThreadGetName(char *buf, int length); +CF_SWIFT_EXPORT int _CFThreadSetName(pthread_t thread, const char *_Nonnull name); +CF_SWIFT_EXPORT int _CFThreadGetName(char *_Nonnull, int length); CF_EXPORT Boolean _CFCharacterSetIsLongCharacterMember(CFCharacterSetRef theSet, UTF32Char theChar); CF_EXPORT CFCharacterSetRef _CFCharacterSetCreateCopy(CFAllocatorRef alloc, CFCharacterSetRef theSet); diff --git a/Foundation/Thread.swift b/Foundation/Thread.swift index dae27cd93a..14899694b3 100644 --- a/Foundation/Thread.swift +++ b/Foundation/Thread.swift @@ -43,6 +43,9 @@ internal enum _NSThreadStatus { private func NSThreadStart(_ context: UnsafeMutableRawPointer?) -> UnsafeMutableRawPointer? { let thread: Thread = NSObject.unretainedReference(context!) Thread._currentThread.set(thread) + if let name = thread.name { + _CFThreadSetName(pthread_self(), name) + } thread._status = .executing thread.main() thread._status = .finished @@ -141,11 +144,12 @@ open class Thread : NSObject { } internal var _main: () -> Void = {} -#if os(OSX) || os(iOS) || CYGWIN - private var _thread: pthread_t? = nil -#elseif os(Linux) || os(Android) +#if os(Android) private var _thread = pthread_t() +#else + private var _thread: pthread_t? = nil #endif + #if CYGWIN internal var _attr : pthread_attr_t? = nil #else @@ -202,8 +206,8 @@ open class Thread : NSObject { open var name: String? { didSet { - if _thread == Thread.current._thread { - _CFThreadSetName(name) + if let thread = _thread { + _CFThreadSetName(thread, name ?? "" ) } } } diff --git a/TestFoundation/TestThread.swift b/TestFoundation/TestThread.swift index f5b224b8e3..5d95146d94 100644 --- a/TestFoundation/TestThread.swift +++ b/TestFoundation/TestThread.swift @@ -59,34 +59,69 @@ class TestThread : XCTestCase { } func test_threadName() { - let thread = Thread() - XCTAssertNil(thread.name) - func getPThreadName() -> String? { - var buf = [Int8](repeating: 0, count: 16) + // Compare the name set in pthreads() + func compareThreadName(to name: String) { + var buf = [Int8](repeating: 0, count: 128) +#if os(OSX) || os(iOS) + // Dont use _CF functions on macOS as it will break testing with Darwin's native Foundation. + let r = pthread_getname_np(pthread_self(), &buf, buf.count) +#else let r = _CFThreadGetName(&buf, Int32(buf.count)) - - guard r == 0 else { - return nil +#endif + if r == 0 { + XCTAssertEqual(String(cString: buf), name) + } else { + XCTFail("Cant get thread name") } - return String(cString: buf) } + // No name is set initially + XCTAssertNil(Thread.current.name) + +#if os(Linux) // Linux sets the initial thread name to the process name. + compareThreadName(to: "TestFoundation") +#else + compareThreadName(to: "") +#endif + Thread.current.name = "mainThread" + XCTAssertEqual(Thread.mainThread.name, "mainThread") + + let condition = NSCondition() + condition.lock() + let thread2 = Thread() { - Thread.current.name = "Thread2" - XCTAssertEqual(Thread.current.name, "Thread2") - XCTAssertEqual(Thread.current.name, getPThreadName()) + XCTAssertEqual(Thread.current.name, "Thread2-1") + compareThreadName(to: "Thread2-1") + + Thread.current.name = "Thread2-2" + XCTAssertEqual(Thread.current.name, "Thread2-2") + compareThreadName(to: "Thread2-2") + + Thread.current.name = "12345678901234567890" + XCTAssertEqual(Thread.current.name, "12345678901234567890") +#if os(OSX) || os(iOS) + compareThreadName(to: "12345678901234567890") +#elseif os(Linux) + // pthread_setname_np() only allows 15 characters on Linux + compareThreadName(to: "123456789012345") +#endif + condition.lock() + condition.signal() + condition.unlock() } - + thread2.name = "Thread2-1" thread2.start() - Thread.current.name = "CurrentThread" - XCTAssertEqual(Thread.current.name, getPThreadName()) + // Allow 1 second for thread2 to finish + XCTAssertTrue(condition.wait(until: Date(timeIntervalSinceNow: 1))) + condition.unlock() + XCTAssertEqual(Thread.current.name, "mainThread") + XCTAssertEqual(Thread.mainThread.name, "mainThread") let thread3 = Thread() thread3.name = "Thread3" XCTAssertEqual(thread3.name, "Thread3") - XCTAssertNotEqual(thread3.name, getPThreadName()) } func test_mainThread() { From ba6bc4b6dd015f303ce529dd95c7c6ef1301e653 Mon Sep 17 00:00:00 2001 From: Simon Evans Date: Mon, 23 Oct 2017 15:39:34 +0100 Subject: [PATCH 2/3] Thread: fix missing parameter name --- CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h b/CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h index 1d0efbc99c..57e2f122b5 100644 --- a/CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h +++ b/CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h @@ -317,7 +317,7 @@ typedef pthread_t _CFThreadRef; CF_EXPORT _CFThreadRef _CFThreadCreate(const _CFThreadAttributes attrs, void *_Nullable (* _Nonnull startfn)(void *_Nullable), void *restrict _Nullable context); CF_SWIFT_EXPORT int _CFThreadSetName(pthread_t thread, const char *_Nonnull name); -CF_SWIFT_EXPORT int _CFThreadGetName(char *_Nonnull, int length); +CF_SWIFT_EXPORT int _CFThreadGetName(char *_Nonnull buf, int length); CF_EXPORT Boolean _CFCharacterSetIsLongCharacterMember(CFCharacterSetRef theSet, UTF32Char theChar); CF_EXPORT CFCharacterSetRef _CFCharacterSetCreateCopy(CFAllocatorRef alloc, CFCharacterSetRef theSet); From 0021cb6c013936a4f833358f72d408c2203b8075 Mon Sep 17 00:00:00 2001 From: Simon Evans Date: Fri, 3 Nov 2017 19:50:15 +0000 Subject: [PATCH 3/3] Thread: Dont trim thread name on Linux --- CoreFoundation/Base.subproj/CFPlatform.c | 6 +----- TestFoundation/TestThread.swift | 5 +++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/CoreFoundation/Base.subproj/CFPlatform.c b/CoreFoundation/Base.subproj/CFPlatform.c index 4e7e0f692f..5aefe20165 100644 --- a/CoreFoundation/Base.subproj/CFPlatform.c +++ b/CoreFoundation/Base.subproj/CFPlatform.c @@ -1323,11 +1323,7 @@ CF_SWIFT_EXPORT int _CFThreadSetName(pthread_t thread, const char *_Nonnull name } return EINVAL; #elif DEPLOYMENT_TARGET_LINUX - // pthread_setname_np will fail if name >= 16 characters - char short_name[16]; - strncpy(short_name, name, 15); - short_name[15] = '\0'; - return pthread_setname_np(thread, short_name); + return pthread_setname_np(thread, name); #endif } diff --git a/TestFoundation/TestThread.swift b/TestFoundation/TestThread.swift index 5d95146d94..6f09c8d6ee 100644 --- a/TestFoundation/TestThread.swift +++ b/TestFoundation/TestThread.swift @@ -103,8 +103,9 @@ class TestThread : XCTestCase { #if os(OSX) || os(iOS) compareThreadName(to: "12345678901234567890") #elseif os(Linux) - // pthread_setname_np() only allows 15 characters on Linux - compareThreadName(to: "123456789012345") + // pthread_setname_np() only allows 15 characters on Linux, so setting it fails + // and the previous name will still be there. + compareThreadName(to: "Thread2-2") #endif condition.lock() condition.signal()