any way to add implementation for wait on all outstanding?

[jrochkind]

The README async example includes:

 # quick & dirty fix to wait for the block to be called asynchronously
sleep 5

This made me think -- how hard would it be to provide api that lets you do:

 client.wait # or call it `client.join`?  or alias both?

To block waiting for any outstanding async requests that may have been called.

Which also makes me think, you need to doc whether a Client instance is thread-safe or not, in what ways. (I suspect it is not currently).

[ostinelli]

The "quick & dirty fix" is for the example only. If you're using async methods, it's obviously because you have a running loop and don't need to sleep. In this context, what would be the benefit of having a client.wait?

Client is thread-safe to my best effort, this is where the most part happens. In the specs you can see concurrent requests (search for "concurrent"). If you think there are reasons why it may not be, I'd love to hear from you.

[jrochkind]

The benefit would be that at a point in the code that you aren't sure if some other point in the code executed async operations, you could still say "just in case there were any, let's wait on them all to complete before closing the client". Or, say you created a client in some method, and did some async requests, and now you're at the end of the method -- but you want to wait for all the async requests you fired off to be done before closing the client and returning from the method -- because you want to return from the method with a closed client, but only once all the requests already fired off have received their responses.

But I haven't actually yet used this code myself, my suggestion is just based on using similar code where I have used or wanted such a thing, so perhaps I'm misunderstanding the use case. I don't entirely understand what you mean by "it's obviously because you have a running loop", so maybe I'm misunderstanding the intended use cases.

Compare to, for example, concurent-ruby's wait_for_termination method on a threadpool -- equivalent to "stop accepting any new requests, then close only once all in-progress requests are complete." Which actually does remind me that wait would have unclear semantics by itself -- what if someone else starts another async request while you are waiting? (Assuming the client is really thread-safe, this could be another thread). The useful semantics would have to be more like that wait_for_termination -- go into a state where no new requests are accepted, wait for all in-progress ones to complete, then close -- with or without the caller blocking to wait for it.

Maybe I'm mistaken about use cases and it's not required though. It is a common pattern in other areas that do concurrency of any kind.

[jrochkind]

Oh, I have no reason to think it wouldn't be thread-safe, I haven't looked at or used the code intensively yet!

Just suggesting the README should make some statements about it's thread-safety, with what sorts of thread-concurrent use is supported, and what sort is not (if any).

Maybe also remind people that the block in the async examples may be executed in another thread (if that's true? i think so?), so needs to be written thread-safely on the caller's part.

[ostinelli]

Maybe also remind people that the block in the async examples may be executed in another thread (if that's true? i think so?), so needs to be written thread-safely on the caller's part.

Yes that's true, and it's a good suggestion.

[ostinelli]

Oh, I have no reason to think it wouldn't be thread-safe

I've just written a test that demonstrates that it is not. Currently working on it. If you're up for the task of helping, let me know :)

[jrochkind]

I'll see if I can find time to take a shot. Want to put the failing test in a branch/PR so people can see it?

I do strongly encourage the use of concurrent-ruby for it's primitives to help make things thread safe.

[ostinelli]

It's in #4 if you get the chance.

[jrochkind]

Cool. And good job writing a test that actually demonstrates a race condition, that's often the hardest part! Concurrent programming is hard.

[ostinelli]

Thread safety resolved, all tests passing.

This thread has been treating different arguments, and your original point is now lost in all of this. If you still think we should consider adding a .join method, would you mind opening another dedicated issue to properly keep track of it? :)

Thank you.

[jrochkind]

I do still think so, but the new one I'd open would just be a copy and paste of this one! I'll let you make the call, or wait until I've spent more time with the library so my suggestion is based on experience.

[ostinelli]

Fair enough. Reopening.

[ostinelli]

This is now in, thank you for your input!