search

nucleic / enaml

Declarative User Interfaces for Python
http://enaml.readthedocs.io/en/latest/
Other
1.53k stars 130 forks source link

Declarative function reference counting issue #417

Closed MatthieuDartiailh closed 4 years ago

MatthieuDartiailh commented 4 years ago

This fixes a bug identified in https://github.com/nucleic/enaml/pull/348, which boils down to a reference counting issue in declarative function that led to the deletion of the builtins dictionary.

codecov-commenter commented 4 years ago

Codecov Report

Merging #417 into master will not change coverage. The diff coverage is 33.33%.

@@           Coverage Diff           @@
##           master     #417   +/-   ##
=======================================
  Coverage   73.47%   73.47%           
=======================================
  Files         311      311           
  Lines       23788    23788           
=======================================
  Hits        17478    17478           
  Misses       6310     6310
MatthieuDartiailh commented 4 years ago

@frmdstryr if you can confirm this fixes the issue it would be great. I am not sure why the double Looper was the only one showing that issue. Basically anything calling dynamic functions enough time can trigger that bug. Meaning it is really dangerous for long running application !

frmdstryr commented 4 years ago

Yes it seems to be working here.

I'll can also test it with another project tomorrow where I originally saw this.

Thank you!

frmdstryr commented 4 years ago

Meaning it is really dangerous for long running application !

Haha, yeah found that out when one of my enaml-web websites crashed after a month or so of running...

Are there any tools that you know of to watch for stuff like this? I know of https://valgrind.org/ but haven't used it enough to be able to do much with it.

MatthieuDartiailh commented 4 years ago

Valgrind can be used to watch for memory leak from my understanding. Here the issue is spurious decref which internal to python. You could in the background of your website run a monitoring of the references and look for suspicious trend (constant increase, or decrease of supposedly long-lived objects). The gc module probably provides all the required tools to do something like that.

cg9999 commented 4 years ago

This fixed the crashing issues I had with inkcut. Thanks!

MatthieuDartiailh commented 4 years ago

Thanks for the confirmation !

frmdstryr commented 4 years ago

It's also no longer crashing in the app where I originally saw it. Nice work!

MatthieuDartiailh commented 4 years ago

Thanks. I will do my best to cut the release and publish the wheels next week. And please do not hesitate to report that kind of issue. Hopefully that was the last one for the cppy update.

MatthieuDartiailh commented 4 years ago

I cut and published the release and I strongly encourage all users to update ! I also audited all uses of function returning borrowed references and hopefully this was the last issue caused by the cppy migration.

MatthieuDartiailh commented 4 years ago

conda package should show up later this week.