Ticket #9513 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

PythonAlgorithm::__del__ method is not called

Reported by: Martyn Gigg Owned by: Martyn Gigg
Priority: critical Milestone: Release 3.3
Component: Python Keywords: Maintenance
Cc: Blocked By:
Blocking: Tester:

Description (last modified by Martyn Gigg) (diff)

This scripts demonstrates that the Python object __del__ method is never called when it should be, indicating that the objects are not being deleted.

from mantid.kernel import *
from mantid.api import *

OBJECT_COUNT = 0

class ObjectTracker(Algorithm):

    def __init__(self):
        global OBJECT_COUNT
        OBJECT_COUNT += 1
        Algorithm.__init__(self)

    def __del__(self):
        global OBJECT_COUNT
        OBJECT_COUNT -= 1
        print "Called __del__"

    def PyInit(self):
        pass

    def PyExec(self):
        print "PyExec"

####################################################################                                                                          
AlgorithmFactory.subscribe(ObjectTracker)
####################################################################                                                                          

OBJECT_COUNT = 0
for i in range(10):
    alg = AlgorithmManager.createUnmanaged("ObjectTracker")
    print "Global object count=%d" % OBJECT_COUNT
    alg.initialize()
    alg.setLogging(False)
    alg.execute()
    del alg

print "Final count",OBJECT_COUNT

The final count should be zero but it will be 10.

Change History

comment:1 Changed 6 years ago by Martyn Gigg

Simplify shared_ptr extraction from Python object

Also avoid using the raw Python C API in favour of the boost python wrappers to ensure that the reference counting is dealt with properly Refs #9513

Changeset: c0c99159c8b164ad2acf60f46ffdc9881228221f

comment:2 Changed 6 years ago by Martyn Gigg

  • Description modified (diff)

comment:3 Changed 6 years ago by Martyn Gigg

Fix PythonAlgorithmTraitsTest

The object returned from the factory now has been downcast to the correct type in Python meaning in the tests the methods are bypassing the base class in some cases. Refs #9513

Changeset: f1c325329f0c8043bcc9644a0fc5113a672fd5a4

comment:4 Changed 6 years ago by Nick Draper

  • Status changed from new to assigned

comment:5 Changed 6 years ago by Martyn Gigg

  • Milestone changed from Release 3.2 to Release 3.3

Agreed with Ross that this is okay in 3.3

comment:6 Changed 6 years ago by Martyn Gigg

  • Keywords Maintenance added

comment:7 Changed 6 years ago by Martyn Gigg

  • Priority changed from major to critical

comment:8 Changed 6 years ago by Martyn Gigg

  • Status changed from assigned to inprogress

Wrap the boost::python deleter with one that locks the GIL first.

For objects that might be deleted when the GIL is not already held already, e.g. live data or a sub algorithm of another C++ algorithm, this avoids a segfault on later versions of Python 2.7. Refs #9513

Changeset: fd72806fa12fd838bc984d8b0a9100cf27029cae

comment:9 Changed 6 years ago by Martyn Gigg

Test script that demonstrates a segfault that happened before the commit in comment #8 was added.

from mantid.kernel import *
from mantid.api import *
from mantid.simpleapi import StartLiveData
import time

class PassThrough(Algorithm):

    def PyInit(self):
        self.declareProperty(MatrixWorkspaceProperty("InputWorkspace", "", Direction.Input))
        self.declareProperty(MatrixWorkspaceProperty("OutputWorkspace", "", Direction.Output))

    def PyExec(self):
        inws = self.getProperty("InputWorkspace").value

        self.setProperty("OutputWorkspace", inws)

####################################################################
AlgorithmFactory.subscribe(PassThrough)
####################################################################

StartLiveData(UpdateEvery=10, Instrument='FakeEventDataListener',
                      StartTime='1990-01-01T00:00:00', AccumulationMethod='Append',
                      PostProcessingAlgorithm='PassThrough', PostProcessingProperties='InputWorkspace=live',
                      AccumulationWorkspace='accum', OutputWorkspace='live')

time.sleep(3)
AlgorithmManager.cancelAll()

comment:10 Changed 6 years ago by Martyn Gigg

  • Status changed from inprogress to verify
  • Resolution set to fixed
  • Description modified (diff)

Branch: bugfix/9513_pyobj_del_not_called

Tester: The script in the original comment should produce a 0 final count and the script in comment:9 should not crash on fedora 20.

comment:11 Changed 6 years ago by Ross Miller

  • Status changed from verify to closed

Merge remote-tracking branch 'origin/bugfix/9513_pyobj_del_not_called'

Full changeset: 3513a74ded2813e63c962c80c37e1bf545a65918

comment:12 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 10356

Note: See TracTickets for help on using tickets.