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

Sometimes ZMQ freezes on sendExpression("quit()") #32

Closed
CSchoel opened this issue Sep 25, 2019 · 9 comments · Fixed by #97
Closed

Sometimes ZMQ freezes on sendExpression("quit()") #32

CSchoel opened this issue Sep 25, 2019 · 9 comments · Fixed by #97
Assignees

Comments

@CSchoel
Copy link

CSchoel commented Sep 25, 2019

Sometimes (maybe 1 in 10 or 1 in 20 cases), ZMQ freezes after sendExpression(omc, "quit()").

The OMC instance shuts down properly, but the call never returns. I suppose that this could be a threading issue on the side of OMC so that quit() sometimes shuts down the process before the answer to the ZMQ message is sent.

The clean solution would probably be to look into the ZMQ handling in the OMC, but a quick fix could also be to simply skip the call to ZMQ.recv for the quit() message like this:

...
if expr == "quit()"
    return ""
end
message=ZMQ.recv(omc.socket)
...

If it seems that this clutters up the implementation of sendExpression too much, it would also be possible to implement quit(omc) as a separate function.

@adeas31
Copy link
Member

adeas31 commented Sep 25, 2019

@arun3688 Isn't it already fixed. See #12 and #29.

@JKRT
Copy link
Member

JKRT commented Sep 25, 2019

@arun3688 @adeas31 I think the issue might be due to the official version of this package has not been updated maybe..

@CSchoel Could you try the code from this repo and verify this to us?

@arun3688
Copy link
Contributor

@adeas31 , Yes this is fixed as John pointed it out, @CSchoel may be using the oder version

@adeas31
Copy link
Member

adeas31 commented Sep 25, 2019

Well looking at the code I think the problem is still there. You only check for the process before send. If we send "quit()" it will kill the process so I think you have to check for it before receive as well.

@arun3688
Copy link
Contributor

@adeas31 i will add it

@JKRT
Copy link
Member

JKRT commented Sep 26, 2019

@arun3688 still, did you release a new version of OMJulia after the fix?

@arun3688
Copy link
Contributor

@JKRT you mean which fix, this issue

@JKRT
Copy link
Member

JKRT commented Sep 27, 2019

@JKRT Did we cover this yesterday?

@CSchoel
Copy link
Author

CSchoel commented Sep 30, 2019

@arun3688 @adeas31 I think the issue might be due to the official version of this package has not been updated maybe..

@CSchoel Could you try the code from this repo and verify this to us?

I already used the latest repository version (commit hash 77e7fe0684c8dc6b60cc668fcf1f2ec89c43581e) for my tests, because I needed the fix for #22 to be able to send the quit() command.

I think @adeas31 is right: The fix for #29 does not work in this case, because the omc process is killed after the process status was checked.

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 a pull request may close this issue.

5 participants