-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[NSThread] Implement main and isMainThread #748
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -57,18 +57,30 @@ private func NSThreadStart(_ context: UnsafeMutableRawPointer?) -> UnsafeMutable | |||
} | |||
|
|||
open class Thread : NSObject { | |||
|
|||
static internal let _mainThread = Thread(thread: pthread_self()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When is this initializer actually called? Is it lazy, so the first to ask for it will be implicitly main?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the same question, but test_mainThreadFirstAccess pass so i decided it's ok.
Is there a better way to guarantee initialization on the main thread?
edit: actually, i'm not sure if it wasn't initialized by another test, i'll check with full toolchain build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Darwin we have a pthread 'non-posix' API to get a handle on the main thread. I would be looking for something similar on other platforms pthread implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't find any api for this, only suggestions to use pthread_self within the main function.
And i found that Linux kernel tests use this technique as well.
I checked current implementation with a complete toolchains build and this code
import Foundation
Thread {
print(Thread.main == Thread.current)
}.start()
sleep(1)
prints false
, so the expected behaviour is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import Foundation
Thread {
print(Thread.isMainThread)
print(Thread.main == Thread.current)
print(Thread.main.isMainThread)
}.start()
sleep(1)
print(Thread.isMainThread)
print(Thread.main == Thread.current)
print(Thread.main.isMainThread)
false
false
true
true
true
true
in both debug and release mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try dispatch_async and calling Thread.main in there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DispatchQueue.global().async {
...
}
has the same output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps for safety sake we could call this in __CFInitializeSwift to ensure the proper initialization? however that does make the assumption the library is loaded on the main thread (Android can break this rule)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -186,10 +199,11 @@ open class Thread : NSObject { | |||
#endif | |||
} | |||
|
|||
open func main() { | |||
_main() | |||
// FIXME: should be a functiun but conflicts with the `class var main` propery |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo in "functiun".. shouldn't it be function?
This PR has gone stale and the functionality was merged under #1162, so I'm going to close it. |
All 4 tests pass on Linux but i didn't enable it because i don't know the reason they were excluded.