Skip to content

Change approach for Unity main thread retrieval #238

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

Closed
semuserable opened this issue Jun 23, 2021 · 3 comments
Closed

Change approach for Unity main thread retrieval #238

semuserable opened this issue Jun 23, 2021 · 3 comments
Assignees

Comments

@semuserable
Copy link
Contributor

semuserable commented Jun 23, 2021

We have the following line in ApplicationAdapter

public bool IsMainThread => Thread.CurrentThread.ManagedThreadId == 1;

It seems we cant rely on ManagedThreadId == 1, suggested approach https://stackoverflow.com/questions/26452609/find-out-if-im-on-the-unity-thread. From Unity answers https://answers.unity.com/questions/62631/i-wonder-about-thread-id-plz-answer-me.html

Also, should we store this info on IApplication or put it somewhere else?

@semuserable
Copy link
Contributor Author

After investigating for a bit, we can "save" the thread in any MonoBehaviour.Start. As a possible approach, we can create GameObject during SentryInitialization.Init, assign our MonoBehaviour and "save the thread" from there (static holder?).

cc @bruno-garcia

@bruno-garcia
Copy link
Member

That sounds like a good plan. We have a MonoBehavior that @bitsandfoxes created. We discussed renaming it to a more generic thing like SentryUnityMonoBehavior or something. And have event we can subscribe/unsubscribe from the SDK to do things.

I see we have SentryBehavior already in. Is that used?

Keeping the reference to the main thread from a MonoBehavior is a good approach. It would need to be nullable and while processing events we'd need consider the fact that the field wasn't set yet (MonoBehavior wasn't called yet or failed to hook)

@semuserable
Copy link
Contributor Author

semuserable commented Jun 29, 2021

SentryBehaviour is not used, no. Will look into it :)

@semuserable semuserable self-assigned this Jun 29, 2021
This was referenced Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants