-
Notifications
You must be signed in to change notification settings - Fork 12
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
Asynchronous request timeout has no effect #26
Comments
…pl for backwards compatibility. Use that method when asynchronous requests timeout. For #26.
I checked in the C++ code to fix this. I added a requestTimeout method to GravityRequestor that has a default implementation to maintain backwards compatibility. That seems to work well. I ran into a snag though with the Java update. We use interfaces for these callback classes, so the java GravityRequestor can't have a default implementation. Java 8 adds support for default implementations in interfaces, but the MATLAB versions that most folks are using still require java 6 or 7. Will investigate options a bit more here, but I don't think there are many async Java requestors out there, so it wouldn't be awful if they had to add this new method. |
Thanks - code looks good to me. What will happen if a response comes in after the requestTimeout has been called? I looked at GravityRequestManager, and it looks to me like it would do the right thing since you delete the request from the map, but don't understand request handling/flow well. |
Good question. I missed cleaning up completely - I should also remove the poll item that referenced this socket from the vector that's used to poll all active sockets. But because the socket is closed and removed from the map, it's behaving correctly - no further responses are received by the requestor if the provider responds late and the provider doesn't know or care that no one is waiting for the response. |
I performed the additional cleanup and added python support. Changes merged in #33. Java support is trickier as noted above, but the current changes will not negatively impact the Java side. Will open that as a separate issue. |
There's a timeout parameter to the asynchronous request method on GravityNode, but it isn't used in the code. The timeout for the synchronous call works fine.
The text was updated successfully, but these errors were encountered: