Opened 8 years ago
Closed 8 years ago
#770 closed defect (fixed)
Memory leak on windows with any model open, 40 Mb / minute
Reported by: | Tom Goddard | Owned by: | pett |
---|---|---|---|
Priority: | major | Milestone: | Alpha2 |
Component: | Core | Version: | |
Keywords: | Cc: | suzuryu9000@…, conrad@… | |
Blocked By: | Blocking: | ||
Notify when closed: | Platform: | all | |
Project: | ChimeraX |
Description
On WIndows with any model (tested PDB, mol2, density map) open ChimeraX daily build from July 27, 2017 consumes memory, about 40 Mb / minute, when the application is shown but with no mouse or keyboard use (completely idle). The memory leak does not happen on Mac. I think Conrad saw this a month ago. Even after closing all open models the memory continues to leak.
Ryan Wong (intern) found this bug and we determined it is caused by
atomic.check_for_changes()
in src/core/atomic/changes.py being called from the graphics update (src/core/updateloop.py). It appears part of the problem is that session.change_tracker.changed always returns True on Windows (even if no atomic models are open). And then somehow creating the Changes object and invoking the "changes" trigger on every 1/60th second timer callback leaks the memory. We did not investigate where that memory is leaked.
Change History (5)
comment:1 by , 8 years ago
follow-up: 2 comment:2 by , 8 years ago
The patch below fixes it on my home machine, just changing the expected return value type of "change_tracker_changed" from "npy_bool" to "ctypes.c_bool". Not sure of the details, but it looked like the C++ side is returning "false" but the Python side was getting "True". Did not dig into ctypes far enough to figure out the difference between the two types. Conrad diff --git a/src/core/atomic/molobject.py b/src/core/atomic/molobject.py index b696156..7a35452 100644 --- a/src/core/atomic/molobject.py +++ b/src/core/atomic/molobject.py @@ -1821,7 +1821,7 @@ class ChangeTracker: reason.encode('utf-8')) @property def changed(self): - f = c_function('change_tracker_changed', args = (ctypes.c_void_p,), ret = npy_bool) + f = c_function('change_tracker_changed', args = (ctypes.c_void_p,), ret = ctypes.c_bool) return f(self._c_pointer) @property On 8/1/2017 10:42 AM, ChimeraX wrote:
comment:3 by , 8 years ago
Do you want me to apply that patch to my repository and push it to plato?
--Eric
follow-up: 4 comment:4 by , 8 years ago
Not yet. There was empirical evidence that the patch changed the behavior. I'd really would rather understand WHY it changed the behavior. Conrad On 8/1/2017 1:18 PM, ChimeraX wrote:
comment:5 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
"Fixed" by 1b849bdc2.
The ChangeTracker.changed method calls a C++ function via ctypes and used a return value type of npy_bool, which is really numpy.bool, which is a Python type. Instead of interpreting the return value as a boolean, this makes ctypes interpret the return value as an integer and pass it as an argument to numpy.bool and uses that return value as the return value of the function call. Since the actual return value is of type boolean, something got messed up in the translation. The fix is to use a return value type of ctypes.c_bool, and ctypes interprets the return from the C++ function properly.
Okay, the memory leak part of this is fixed. Now the "track changes constantly true" part needs investigation, probably later today after a bunch of demos in the room I would work in are over.
--Eric