Changes between Initial Version and Version 1 of Ticket #17320, comment 8


Ignore:
Timestamp:
11/13/14 07:37:59 (6 years ago)
Author:
nbruin
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #17320, comment 8

    initial v1  
    22> I do not understand why you put all this in a "try" block. Wouldn't it be easier to just set `self.si` and `self.model` to NULL at the beginning, and add  "if self.model != NULL" in the dealloc ?
    33
    4 Quite possibly, but that's not how that code was written and I wasn't planning on delving into the program logic. There's a "new ..." there and there are uncontrolled exits via explicit raises and quite possibly by other code that can generate errors. If the new model gets inserted into self.model there's no leak because the old value of self.model gets properly deleted, but all the error condition exits would lead to an allocated "model" that simply falls out of scope, i.e., a memory leak (unless cython generates an implicit try/finally to cleanly delete any local variables that point to C++ objects).
     4Quite possibly, but that's not how that code was written and I wasn't planning on delving into the program logic. There's a "new ..." there and there are uncontrolled exits via explicit raises and quite possibly by other code that can generate errors. If the new model gets inserted into self.model there's no leak because the old value of self.model gets properly deleted, but all the error condition exits would lead to an allocated "model" that simply falls out of scope, i.e., a memory leak (unless cython generates an implicit try/finally to cleanly delete any local variables that point to C++ objects--it doesn't, otherwise the original code wouldn't have worked properly).
    55
    66I just expressed it as a try/finally to ensure we always have a chance to see if "model" needs to be deleted. I'm sure there are other solutions.