-
Notifications
You must be signed in to change notification settings - Fork 162
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
Work around open-telemetry/opentelemetry-swift#615 #667
base: main
Are you sure you want to change the base?
Work around open-telemetry/opentelemetry-swift#615 #667
Conversation
… Static Linux SDK
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 am not sure I want the block change also on non Linux platforms, I don't feel very safe changing existing behavior in users, thinking about memory and circular dependencies here. Could it be changed to only affect Linux?
…ad subclass on Apple platforms
@nachoBonafonte Understood! I went ahead made that change. I also added some tests to try to catch any reference cycle issues. |
Please take a look at this test that is failing. |
#615 is still an issue when building for Linux, and the PR in the swift project appears to have stalled. This PR makes the subclass of
Thread
a plain class with aThread
property that spawns a thread with a closure instead. This appears to work fine on macOS and Linux in my limited testing so far.I'm not sure if this PR should actually be merged, ideally the error would be resolved in Swift.
I also made a couple minor changes to support building with the Static Linux SDK as that made checking for Linux only compiler errors considerably easier.