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

Manage images with unique_ptr #736

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Conversation

tobiolo
Copy link
Collaborator

@tobiolo tobiolo commented Oct 31, 2024

ThreadPool cannot deal with unique_ptr in a straight-forward way, so leave it out for now.

@tobiolo tobiolo merged commit 2c0d51f into aardappel:master Oct 31, 2024
4 checks passed
@tobiolo tobiolo deleted the unique_ptr-images branch October 31, 2024 15:31
@aardappel
Copy link
Owner

Wait, can you please explain? I doubt there is any problem with unique_ptr and threads, there is likely a different issue.

@tobiolo
Copy link
Collaborator Author

tobiolo commented Oct 31, 2024

The problem is it seems is that I cannot pass the unique pointer reference into the lambda function of pool.enqueue. The compiler gives me some errors about std::invoke, ::type mismatch etc.

@tobiolo
Copy link
Collaborator Author

tobiolo commented Oct 31, 2024

Sorry, I could have waited.

@aardappel
Copy link
Owner

Ok, but those errors can probably be solved. I am not sure why that would warrant removing all the multi-threading stuff, which presumably was useful.

In general, you use unique_ptr to represent the pointer that OWNS the object, not necessarily for passing pointers. It is fine to pass as T * as long as you're sure that the unique_ptr outlives it.

@tobiolo
Copy link
Collaborator Author

tobiolo commented Oct 31, 2024

You are perfectly fine. This is why I would like to reintroduce ThreadPool with the other PR. I now found a way to do it and your hint was the one I needed. 🙏

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.

2 participants