Skip to content

Conversation

@ComputerDuck
Copy link

Change std::sync::Mutex to tokio::sync::Mutex so the MutexGuard implements Send to make the Client usable in an async context and change some function signartures to be async

Change std::sync::Mutex to tokio::sync::Mutex so the MutexGuard implements Send to make the Client usable in an async context and change some function signartures to be async
@SpamapS
Copy link
Owner

SpamapS commented May 31, 2025

Hey hey, cool idea! Unfortunately this breaks a lot of tests. I am poking at your branch now though, I think it may ultimately be a better way to handle the client, and the tests may actually be the problem.

@SpamapS
Copy link
Owner

SpamapS commented May 31, 2025

So, I think the complication mainly arises because of the way can_do's func argument implements in-worker status update methods. Because we didn't have async closures when it was written, we could only take non-async functions to execute. This was not ideal, but it did work ok because we could just spawn a new runtime if we needed to send updates back to the server before the function finished, it would just have to block on those sending inside the worker. You can see that in test_status_updates.

But with the change to a fully async client, we're now going to be blocking the same thread that would execute the runtime, and the code correctly panics in this situation, because we'll never be able to return from that block otherwise.

So, I think in order to make this work right we'd need to also adapt can_do to take an async closure so that func can execute in the same runtime as the worker itself. I think that would ultimately also be a much better experience for worker writers and is more like how the C/C++ libgearman works.

I will noodle on how we might do that.. but I think it's probably worth breaking can_do, as long as we can make it clear that workers are easier to write now, it's a doable thing.

@SpamapS
Copy link
Owner

SpamapS commented Jun 1, 2025

I pushed one possible solution to https://github.com/SpamapS/rustygear/tree/impl_send_for_client_fixes but I really think async closures are going to make this the most ergonomic, even though they'll require the most recent stable Rust.

@ComputerDuck
Copy link
Author

Okay that sounds great. Didn't realize that this would break any tests. I actually used my changed version in a project and it worked fine for me. I have actually decided to write a custom gearman client called gearrs. It's under MIT license, if you want to check it out, but I would also still be happy to help on this project, although I don't really have a great overview over your codebase.

@SpamapS
Copy link
Owner

SpamapS commented Jul 20, 2025

Okay that sounds great. Didn't realize that this would break any tests. I actually used my changed version in a project and it worked fine for me. I have actually decided to write a custom gearman client called gearrs. It's under MIT license, if you want to check it out, but I would also still be happy to help on this project, although I don't really have a great overview over your codebase.

Yeah it would only panic if you tried to send real-time status updates to clients from a worker function. It's not a common use-case.

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