Skip to content
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

Fix QTimer being created on incorrect thread after refresh. #1893

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

Holt59
Copy link
Member

@Holt59 Holt59 commented Oct 2, 2023

No description provided.

@Silarn
Copy link
Member

Silarn commented Oct 2, 2023

Interesting. I know that the signal stuff can work across threads but I'm not sure why having a lambda would trigger creating a thread.

@Holt59
Copy link
Member Author

Holt59 commented Oct 2, 2023

Interesting. I know that the signal stuff can work across threads but I'm not sure why having a lambda would trigger creating a thread.

It's not 100% clear in the documentation, but with the lambda version, the context is the sender, so the functor is called in the sender context, which is a separate thread (since refresh is done in a separate thread). If you add this, with a lambda or a function-pointer, the context is now this so the call ends-up in the right thread.

@Silarn
Copy link
Member

Silarn commented Oct 2, 2023

Yeah, that makes sense, I think.

@AnyOldName3
Copy link
Member

While this is probably the best way to fix this specific problem, if it comes up again in the future, you can be more explicit with how Qt should deal with threads when signals need wiring to slots. There are several different examples in the OMOD installer as .NET wanted to do its own things on its own threads, and Qt wasn't happy about it.

@Silarn Silarn merged commit b5ba852 into master Oct 3, 2023
3 checks passed
@Holt59 Holt59 deleted the fix/directory-refreshed-qtimer branch October 3, 2023 16:10
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

Successfully merging this pull request may close these issues.

3 participants