search

BruceSherwood / vpython-jupyter

This repository has been moved to https://github.com/vpython/vpython-jupyter
64 stars 33 forks source link

Replaced calls to os._exit(0) in no_notebook with graceful shutdown #102

Closed pmelanson closed 5 years ago

pmelanson commented 5 years ago

The code in this PR is slightly different from the diff I posted in the google groups discussion, to take into account changes that came into master from mwcraig/this-will-be-the-one.

The code still works, however this code may have uncovered a minor bug in the vpython-glowscript communication logic. Specifically, now that I don't call os._exit(0), it seems the following __del__ functions get called that weren't getting called before and throw exceptions on vpython exit:

Traceback (most recent call last):
  File "C:\...\site-packages\vpython\vpython.py", line 306, in __del__
NameError: name 'sender' is not definedException ignored in: <function baseObj.__del__ at 0x000001D765772E18>
Traceback (most recent call last):
  File "C:\...\site-packages\vpython\vpython.py", line 306, in __del__
NameError: name 'sender' is not defined
Exception ignored in: <function standardAttributes.__del__ at 0x000001D76577A598>
Traceback (most recent call last):
  File "C:\...\site-packages\vpython\vpython.py", line 1092, in __del__
  File "C:\...\site-packages\vpython\vpython.py", line 306, in __del__
NameError: name 'sender' is not defined
Exception ignored in: <function standardAttributes.__del__ at 0x000001D76577A598>
Traceback (most recent call last):
  File "C:\...\site-packages\vpython\vpython.py", line 1092, in __del__
  File "C:\...\site-packages\vpython\vpython.py", line 306, in __del__
NameError: name 'sender' is not defined
Exception ignored in: <function baseObj.__del__ at 0x000001D765772E18>
Traceback (most recent call last):
  File "C:\...\site-packages\vpython\vpython.py", line 306, in __del__
NameError: name 'sender' is not defined
Exception ignored in: <function standardAttributes.__del__ at 0x000001D76577A598>
Traceback (most recent call last):
  File "C:\...\site-packages\vpython\vpython.py", line 1092, in __del__
  File "C:\...\site-packages\vpython\vpython.py", line 306, in __del__
NameError: name 'sender' is not defined

I poked around a little to try and get these errors to go away, but I didn't want to randomly poke even more vpython internals. It looks to be a simple change to squash this error, I just don't know what the change should be.

Thanks for looking at this PR Matt!

mwcraig commented 5 years ago

@pmelanson -- thanks for the PR!

It looks like sender is only set when running in a notebook but gets called in the delete methods regardless of whether display is in the notebook or not, so this is definitely a bug.

@BruceSherwood -- would it be better to fix this by setting sender to something appropriate in the no-notebook case or by changing the logic at https://github.com/BruceSherwood/vpython-jupyter/blob/master/vpython/vpython.py#L303 and similar?

It looks like a global sender = None was removed in 417b237c86cd0db4d5bb1daaabc9ba94aac406b9

BruceSherwood commented 5 years ago

I honestly don't know, Matt. I think that the del code has not been really tested.

mwcraig commented 5 years ago

I'm going to go ahead and merge this...in a moment I'll push a fix to the errors.

I'll also start a file with a list of contributors and add @pmelanson to it -- thanks again for this fix!

pmelanson commented 5 years ago

Thanks Matt!