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

Quitting does not release ctx #12

Closed
rpapallas opened this issue Jul 21, 2022 · 5 comments
Closed

Quitting does not release ctx #12

rpapallas opened this issue Jul 21, 2022 · 5 comments

Comments

@rpapallas
Copy link
Contributor

When ESC is pressed to terminate the viewer, the code will just:

print("Prssed ESC")
print("Quitting.")
glfw.terminate()
sys.exit(0)

Is there a reason why this code is not just calling the self.close() which does partly the same and in addition releases ctx?

@rohanpsingh
Copy link
Owner

I assumed that ctx is freed when sys exit is called. We can call self.close() and then do a sys exit too. Are there any disadvantages you see in the current implementation?

@rpapallas
Copy link
Contributor Author

I think the sys.exit is the problem, not that we don't release ctx. I think when sys.exit from the viewer, the client code will terminate ungracefully. For example, on macOS, when the viewer is running and some other code is running in a separated thread, ESC (i.e., sys.exit) will not wait for the thread of the client code to finish and this gives an error. It might be better not to sys.exit in the viewer, or pass to the viewer a clean-up client code that will run when terminating before sys.exit (although this doesn't seem clean)?

@rpapallas
Copy link
Contributor Author

rpapallas commented Jul 23, 2022

This is the solution I propose: rpapallas@2110f91

Let me know what you think. The client needs to do the following:

with MujocoViewer(model, data) as viewer:
    while viewer.is_running:
        viewer.render()

If not, the program will still terminate due to NotImplemenetedError error.

@rohanpsingh
Copy link
Owner

Hi @rpapallas
Originally, I inserted the sys.exit on purpose because I did want to force kill the thread if I pressed ESC. But I agree this may not be the behavior most people want (and I never tried this on macOS).

Thanks for the PR but it causes a segfault for me on Ubuntu when I press ESC, which I think it should.
So I created a similar mechanism to exit on this branch, which seems to work fine. Could you please try this out and let me know?

@rpapallas
Copy link
Contributor Author

Hi @rohanpsingh I can confirm your fix works fine on macOS, thank you!

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