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

Non-blocking version of nng_aio_wait / nng_aio_result #1574

Closed
shikokuchuo opened this issue Feb 17, 2022 · 6 comments
Closed

Non-blocking version of nng_aio_wait / nng_aio_result #1574

shikokuchuo opened this issue Feb 17, 2022 · 6 comments

Comments

@shikokuchuo
Copy link
Contributor

Porting our conversation from Discord:

I am coding a function for the next release of nanonext (R binding to NNG) that peeks at the result of an Aio without waiting for it to complete. I have a solution but it involves querying from another thread and quite a bit of workaround to make it seamless.

This is relevant for interpreted languages such as R or Python where the interpreter runs only on the main thread and it is dangerous to have anything blocking (unintentionally), especially as there is no ability to watch for EINTR. Thus calling nng_aio_wait without knowing if the operation has completed can be undesirable at times even if that is the usual intention.

It occurred to me that this would be trivial if nni_aio->a_result were initialised with something other than 0 so that it can be readily distinguished from the success exit code. This way we could just call nng_aio_result to know the result at any time (without having to call nng_aio_wait first).

Your initial reply:

This may be reasonable although I’m distinctly concerned about polling - there is an ownership issue here that needs consideration. It may be that the aio api can be extended to give you a way to poll for completion status. Sort of a non blocking wait.

@shikokuchuo
Copy link
Contributor Author

Adding some more context:

As an example, using req/rep in an RPC client/server set-up, where the requestor is a critical task that sends information periodically to the server. Before each request, it checks the status of the previous request by calling its recv aio = nng_aio_wait then nng_aio_get_message. At this point, you would not want a blocking wait, so a non-blocking method of querying the completion status would seem to be useful.

@shikokuchuo
Copy link
Contributor Author

So I now have a proper implementation using the aio callback and a mutex.

This still feels it should be unnecessary given the result is already there in your aio struct, just it is not possible to distinguish a 0 success value from the initialised value. Is it as simple as initialising the result as say NNG_EUNRESOLV = 32 and using a mutex around calls to the result, rather than requiring a new interface?

@shikokuchuo
Copy link
Contributor Author

I take back my last message and concur that it is best to have another interface to query completion status. Possibly nni_aio_finish_impl() can write a completion bit (byte), and then just a public nng_ function to read that.

@gdamore
Copy link
Contributor

gdamore commented Apr 16, 2022

I think we already have most of what we need without implementing that.

Note that your callbacks could also write to some state if you needed to.

What I'll probably do is implement an nng_aio_busy() function that returns true if the aio is still in use. It will only return false once the aio is complete, and all callbacks are done.

Note that it would be a gross error to free the aio and then subsequently call this check. So care will need to be taken on the part of the consumer to ensure that the aio is not subject to use-after-free.

@shikokuchuo
Copy link
Contributor Author

Yes, that should work.

The callback is what I currently use. Each aio is wrapped in a struct that has a completion byte and nng mutex among other things. The last action of the aio callback writes the completion status (acquiring the mutex). All functions that attempt to retrieve a message/result/iov etc. then read this status first (acquiring the mutex), proceeding only if safe to do so.

Use-after-free would not be an issue from a binding perspective - references to freed aios are simply not kept around.

gdamore added a commit that referenced this issue Apr 18, 2022
This introduces a new API, nng_aio_busy(), that can be used
to query the status of the aio without blocking.

Some minor documentation fixes are included.
@shikokuchuo
Copy link
Contributor Author

Thanks for implementing. This is now incorporated in the dev version of nanonext shikokuchuo/nanonext@3b73ede

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