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

Fix observed hangs with LV measurements for certain situations of service launches #266

Merged
merged 5 commits into from
Sep 11, 2023

Conversation

jonathanmendez
Copy link
Contributor

@jonathanmendez jonathanmendez commented Sep 8, 2023

What does this Pull Request accomplish?

Fixes AB#2508868 and AB#2464807

Why should this Pull Request be merged?

In the existing implementation of the service startup code, there is a race condition caused by a delay between registering the measurement service with Discovery Service (enabling clients to try to call the measurement service) and registering the user events for handling calls to the measurement service (enabling the measurement service to receive and react to calls). If a client makes a call to the measurement service between those two pieces of code, the measurement service never handles the call and the client waits indefinitely.

The fix moves the event registration before the measurement is registered with Discovery Service. The events are registered and the registration refnum is stored in the service private data to later be wired to the event handler. Through testing, the registration call is sufficient to be ready to handle calls, even if the event handler structure has not been reached yet.

What testing has been done?

Tested the implementation using a reproducing TestStand sequence that repeatedly calls and kills a measurement. Before the fix, a hang would consistently occur within a few minutes. After the fix, I have not observed a hang over many minutes and repeated runs.

…data to use

Signed-off-by: Jonathan Mendez <jonathan.mendez@ni.com>
Signed-off-by: Jonathan Mendez <jonathan.mendez@ni.com>
@dixonjoel
Copy link
Collaborator

Would you be able to post some screenshots for the relevant changes? I'm not sure if I need to look at all these files or if most are just type prop that had to be saved.

@pbirkhol-ni
Copy link
Contributor

Would you be able to post some screenshots for the relevant changes? I'm not sure if I need to look at all these files or if most are just type prop that had to be saved.

I found that the only VIs with real changes are:

  • Register gRPC Methods and Messages.vi
  • Start Sync.vi

@jonathanmendez
Copy link
Contributor Author

Would you be able to post some screenshots for the relevant changes? I'm not sure if I need to look at all these files or if most are just type prop that had to be saved.

Yes, I'm working on that now. Unfortunately I ran out of time at my computer as I was posting the PR earlier.

@jonathanmendez
Copy link
Contributor Author

jonathanmendez commented Sep 9, 2023

Here are screenshots for the original changes (before reflecting Paul's review). Only the V1 VIs are shown but the same changes were made to V2.

Class private data:
image

Register gRPC Methods and Messages:
image

Start Sync (before/after):
image
image

The rest are just recompiles due to adding data to the class.

Signed-off-by: Jonathan Mendez <jonathan.mendez@ni.com>
Signed-off-by: Jonathan Mendez <jonathan.mendez@ni.com>
Signed-off-by: Jonathan Mendez <jonathan.mendez@ni.com>
@jonathanmendez jonathanmendez merged commit 609636b into main Sep 11, 2023
1 check passed
jasonmreding added a commit that referenced this pull request Sep 13, 2023
* main:
  Fix observed hangs with LV measurements for certain situations of service launches (#266)
@jonathanmendez jonathanmendez deleted the users/jomendez/hang-bug branch September 19, 2023 20:28
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.

3 participants