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 pett, 8 years ago

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

in reply to:  2 ; comment:2 by Conrad Huang, 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 pett, 8 years ago

Do you want me to apply that patch to my repository and push it to plato?

--Eric

in reply to:  4 ; comment:4 by Conrad Huang, 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 Conrad Huang, 8 years ago

Resolution: fixed
Status: assignedclosed

"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.

Note: See TracTickets for help on using tickets.