-
Notifications
You must be signed in to change notification settings - Fork 60
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
The problem with using Occurrences (for eventual Async Client Calls) #207
Comments
BTW, Async Client-side code is far more interesting to me than the "Async" Server-side stuff that your scripted Server seems to have. I would actually recommend you drop that stuff as over-complicated and something that a User can do a number of ways if they need it. Client-side is far more important to think about. |
@drjdpowell Supporting parallel client requests only requires the VI you showed to be reentrant. Each reentrant instance will get its own occurrence so it is fine. Supporting an async G client API is a different thing and I'm not sure it is warranted. My feeling is that in the vast majority of cases, a reentrant synchronous API is fine. For cases that really need an async G API then async call by ref can be used to wrap the API. If you think it is important to support an async G client API, then I'd say make an issue supporting that enhancement with use cases and this is just an implementation detail of that support. FYI @eyesonvis |
That's why I think you should drop it. It isn't bringing enough to the party to justify the increased difficulty in understanding. Most real-world applications are likely to have a one or two Procedures that take significant time, plus several very quick ones. Imagine, for example, an "Analyze Image" call; Users will want to analyze images in parallel, not to be able to other procedure types in parallel. Also, doing stuff in parallel is a problem that has been solved in multiple ways already. You don't need to have that in scope. |
Some problems there: Anyway, my point is that is complicated, and causes a lot of design restrictions. I actually figured out last night how to work around the Occurrence issue, to get a Malleable API for an Async Client call, but I really had to call on some deep knowledge and experience to do so. It would be a lot easier if you used a Queue. |
Do you know any Actor Framework guys? Ask them what they think about blocking synchronous calls. |
I believe this is behind #272 and #320, and possibly #77. Local loopback can go quickly enough that the reply comes and the occurrence gets fired before LabVIEW execution can resume and get to the occurrence. The result is a deadlock after a race condition. I just noticed that there is a timeout that is unwired - I will submit that as a pull request since it (clunkily) prevents deadlock. It waits out the timeout and then can resume. After seeing all this, I'm persuaded by @drjdpowell - we should strongly consider rewriting the client side. I like the idea of having a higher level call that makes everything easy. But
|
Note: swapping out the Occurrence for a temporary Queue or User Event should fix multiple problems without requiring more than minor changes. |
The Client Sync Calls are well set up to eventually support an Async Client API, as they basically have "Start Action" parts, a "Wait till done" part, and a "Read Result part". However, the use of an Occurrence for the wait part will cause a bug whenever a new Request is made while an older one is not yet filled. This is because the Occurrence is static, and the same on each call, so the Response to the first Request will end the wait for both, causing the read of the second Response to be too early and thus fail.
Normally, I would use a Queue for these types of Waits, creating a new temporary Queue for each Request-Response, allowing multiple such async calls to be "inflight" at any one time. Would it be easy to switch to using a Queue?
The text was updated successfully, but these errors were encountered: