bugGNU Octave - Bugs: bug #53844, classdef: delete destructor method...

 
 

bug #53844: classdef: delete destructor method cannot access class properties

Submitter:  None
Submitted:  Sat 05 May 2018 11:57:01 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Michael Quellmalz Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 4.4.0
Operating System:  * Microsoft Windows Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 21 Feb 2019 11:04:53 PM UTC, comment #20: 

The delete method can access class properties when an object is cleared now, but the issue in comment #13 still stands in Octave 5. I don't get a segmentation fault any more. I will file a new report for the remaining issue of a delete method breaking temporary inline objects, just to keep the history here clean.

Mike Miller <mtmiller>
Group Member
Tue 22 May 2018 10:56:23 AM UTC, comment #19: 

Could this be related to a bug #53956 I recently submitted here:

https://savannah.gnu.org/bugs/?53956

I was having an issue where my delete destructor was clearing variables, then trying to do it again because the delete destructor is incorrectly called multiple times.


Richard <crobar>
Tue 15 May 2018 08:03:03 PM UTC, comment #18: 

I undid the changes here:

http://hg.savannah.gnu.org/hgweb/octave/rev/61ba501d8f04

and merged stable with default.

This fix for classdef will just have to wait until it can be done properly on default.

John W. Eaton <jwe>
Group administrator
Tue 15 May 2018 02:15:15 AM UTC, comment #17: 

I see a stack trace that begins with:


Thread 1 "octave-cli" received signal SIGSEGV, Segmentation fault.
0x0000555555c675a8 in ?? ()
(gdb) where
#0  0x0000555555c675a8 in ?? ()
#1  0x00007ffff71cf688 in cdef_object_rep::release (this=0x555555936b50, obj=...) at /home/jwe/src/octave-stable/libinterp/octave-value/ov-classdef.cc:1227
#2  0x00007ffff6def0a1 in cdef_object::~cdef_object (this=0x555555949600, __in_chrg=<optimized out>) at /home/jwe/src/octave-stable/libinterp/octave-value/ov-classdef.h:219
#3  0x00007ffff71febc6 in octave_classdef::~octave_classdef (this=0x5555559495f0, __in_chrg=<optimized out>) at /home/jwe/src/octave-stable/libinterp/octave-value/ov-classdef.h:1456
#4  0x00007ffff71febee in octave_classdef::~octave_classdef (this=0x5555559495f0, __in_chrg=<optimized out>) at /home/jwe/src/octave-stable/libinterp/octave-value/ov-classdef.h:1456
#5  0x00007ffff6deead6 in octave_value::~octave_value (this=0x7fffffffcd28, __in_chrg=<optimized out>) at /home/jwe/src/octave-stable/libinterp/octave-value/ov.h:325
#6  0x00007ffff7378463 in octave::tree_evaluator::deferred_delete_stack::pop_frame (this=0x5555557d7be0) at /home/jwe/src/octave-stable/libinterp/parse-tree/pt-eval.h:96
#7  0x00007ffff738abda in octave::action_container::method_elem<octave::tree_evaluator::deferred_delete_stack>::run (this=0x555555bde4b0) at /home/jwe/src/octave-stable/liboctave/util/action-container.h:150


Frames #3 and #4 are suspicious since there appears to be a recursive call to the octave_classdef destructor for the same object.

I noticed that the classdef code has some member functions that do "delete this".  That seems like bad form to me, or risky, or at the very least, fragile.

At this point, I'm beginning to think that we should just revert the change that started all of this (http://hg.savannah.gnu.org/hgweb/octave/rev/b328ff3ce0f7) and I will apologize profusely to everyone for my mistake in pushing it so close to the release, and attempt to atone for that by doing what I can to make the classdef code better for version 5.

John W. Eaton <jwe>
Group administrator
Tue 15 May 2018 01:54:21 AM UTC, comment #16: 

Segmentation fault seems to be introduced by my debug code, example in the context of the toyclass:


    function delete (self)
      if (isinteger (self.value))
        fprintf (stderr, 'int\n');
      endif
      fprintf (stderr, '');
    endfunction


With a destructor like this, Octave segfaults, not sure why because I don't have a stack, gdb says Octave has exited.

Mike Miller <mtmiller>
Group Member
Tue 15 May 2018 01:37:16 AM UTC, comment #15: 

Hmm, I get a segmentation fault now with Python. I don't have a minimal test case at the moment, but I can try to narrow this down. Definitely introduced by this latest change though.

Mike Miller <mtmiller>
Group Member
Tue 15 May 2018 12:49:54 AM UTC, comment #14: 

I pushed the following additional changeset:

  http://hg.savannah.gnu.org/hgweb/octave/rev/2f3a66d7cf8a

It seems to fix the problem for me.  Can you think of any other cases where this kind of thing might happen?

John W. Eaton <jwe>
Group administrator
Mon 14 May 2018 11:53:00 PM UTC, comment #13: 

Thank you, this improves things quite a bit. I still have one more case where this deferred deletion does not work, where a temporary instance of a class is deleted too soon.

Consider the following toy class


classdef toyclass < handle

  properties
    value
  endproperties

  methods

    function self = toyclass (x)
      if (nargin < 1)
        x = 0;
      endif
      self.value = x;
    endfunction

    function delete (self)
      fprintf (stderr, '');
    endfunction

    function ret = subsref (self, idx)
      ret = self.value;
    endfunction

  endmethods

endclassdef


And the following examples


>> x = toyclass;
>> x.value
ans = 0
>> class (toyclass)
ans = toyclass
>> size (toyclass)
ans =

   1   1

>> toyclass ().value
>>


In the last statement, the constructor returns a valid object, but it is deleted before the subsref is done successfully on the temporary value.

As before, if there is no delete method, or if the body of the delete method is empty, this problem does not appear.

Mike Miller <mtmiller>
Group Member
Mon 14 May 2018 07:32:39 PM UTC, comment #12: 

In practice, I doubt any external code is instantiating a tree_evaluator or dealing with octave_classdef objects directly. I used codeseach.debian.net and didn't find any. I think I'm fine with either solution.

I haven't gotten around to building and testing this yet, will report back a little later.

Mike Miller <mtmiller>
Group Member
Mon 14 May 2018 06:53:44 PM UTC, comment #11: 

I pushed the following changeset on stable:

  http://hg.savannah.gnu.org/hgweb/octave/rev/97f1d513aaf6

This change (and also http://hg.savannah.gnu.org/hgweb/octave/rev/0548e32e6b27) modifies a public interface so it might not be possible for some .oct files compiled for 4.4.0 to work with newer 4.4.x releases, or for .oct files compiled for a future 4.4.x release to work with 4.4.0.  But as I understand it, that would be limited to code that actually uses these fairly obscure interfaces.  Should we care about that?  If so, then I'm willing to defer this change until version 5, but then I think we should just completely disable automatically calling the delete method for classdef handle objects in any future 4.4.x releases.  Comments?

John W. Eaton <jwe>
Group administrator
Sun 13 May 2018 02:33:26 PM UTC, comment #10: 

The attached patch is the best I've come up with so far.  It will defer deletion of temporary objects created as part of argument list evaluation until the end of the containing statement.  In this patch, all temporary objects are preserved.  It would be better to only hold on to classdef handle objects, but I don't see a way to check whether an octave_value object is a classdef handle object.  We can add that, but it will change interfaces.  We could make this fix on stable and the better one on default.



(file #44147)

John W. Eaton <jwe>
Group administrator
Sat 12 May 2018 09:42:59 PM UTC, comment #9: 

Yeah, I understand why it is happening but I'm not sure what the appropriate fix is yet.

The problem is that in this case, the delete method is called when there is a pending result in the evaluator.  Then any expression that is evaluated as part of the delete method wipes out the pending value.

I also noticed another issue with delete.  The Matlab docs say that for


x = some_handle_object ();
y = x;
delete (x);


both y and x should be deleted but Octave doesn't do that.  Maybe this problem should have a separate report.

John W. Eaton <jwe>
Group administrator
Fri 11 May 2018 11:26:04 PM UTC, comment #8: 

Yeah, it doesn't even have to be a method, and the function doesn't even have to access the object at all, it just has to be on the argument list. So this is probably down in the evaluator, and something to do with the object being destroyed affecting the result of the overall expression.


function r = somefunc (varargin)
  r = 12
endfunction

## No semicolon on the assignment so it's obvious that r is set

>> x = dummy;
>> somefunc (x)
r =  12
ans =  12
>> somefunc (pi)
r =  12
ans =  12
>> somefunc (pi, x)
r =  12
ans =  12
>> somefunc (pi, dummy)
r =  12
>>


Mike Miller <mtmiller>
Group Member
Fri 11 May 2018 11:21:51 PM UTC, comment #7: 

Confirmed.  I modified the file dummy.m to have keyboard statements for each method and then I was able to trace things through.  For some reason, having any sort of delete method is causing output to be skipped.


classdef dummy < handle

  properties
    id
  endproperties

  methods

    function self = dummy (id)
      keyboard;
      if (nargin < 1)
        id = 'dummy';
      endif
      self.id = id;
    endfunction

    function delete (self)
      keyboard;
      # fprintf (stderr, '');
    endfunction

    function r = isa (self, klass)
      keyboard;
      r = isequal (self.id, klass);
    endfunction

  endmethods

endclassdef



Rik <rik5>
Group administrator
Fri 11 May 2018 10:59:14 PM UTC, comment #6: 

And this behavior is the same with Octave 4.4 (d703d8bbc6ae), this was not introduced by the changes on stable since 4.4. But this did work prior to the change to start calling the delete method.

Mike Miller <mtmiller>
Group Member
Fri 11 May 2018 10:54:38 PM UTC, comment #5: 

Ok, here is the strange case that I am now running into. Consider the following toy class


classdef dummy < handle

  properties
    id
  endproperties

  methods

    function self = dummy (id)
      if (nargin < 1)
        id = 'dummy';
      endif
      self.id = id;
    endfunction

    function delete (self)
      fprintf (stderr, '');
    endfunction

    function r = isa (self, klass)
      r = isequal (self.id, klass);
    endfunction

  endmethods

endclassdef


Now the following code at the interpreter prompt shows the problem


>> x = dummy;
>> isa (x, 'dummy')
ans = 1
>> isa (dummy, 'dummy')
>>


Notice how the second call to 'isa' returns nothing at all.

If the delete method is removed, or if the method stays but the entire function body is deleted or commented out, then this behaves as it should. But if the destructor does something, then the method on the transient value returns nothing.

Mike Miller <mtmiller>
Group Member
Fri 11 May 2018 10:09:36 PM UTC, comment #4: 

Thank you for taking a look at this. With this change things are improved slightly, and the simple test you added works for me. I'm seeing some strange problems now, particularly with transient objects, that I will have to dig into further before I can fully identify.

If you want you can close this as fixed and I will report another bug if I find something.

Mike Miller <mtmiller>
Group Member
Fri 11 May 2018 09:02:38 PM UTC, comment #3: 

I pushed the following changeset on stable:

http://hg.savannah.gnu.org/hgweb/octave/rev/2dad85fe6b8b

It seems to work correctly for me now.  I also added a test, but forgot to do that until after I'd already pushed the above changeset.

http://hg.savannah.gnu.org/hgweb/octave/rev/2205c0ca02e7

John W. Eaton <jwe>
Group administrator
Wed 09 May 2018 10:04:46 PM UTC, comment #2: 

Confirmed with the py class, which I haven't tested for a while.

I had a delete method that worked, although it wasn't called automatically, and now it generates an error every time an object is cleared. I would say the current state is a bit worse than the delete method not being called at all.

Mike Miller <mtmiller>
Group Member
Sun 06 May 2018 12:28:01 AM UTC, comment #1: 

I would guess that your analysis is correct.  Using 'clear h' probably partially destroys the object 'h' and then calls the destructor for h.

Rik <rik5>
Group administrator
Sat 05 May 2018 11:57:01 PM UTC, original submission:  

The delete destructor method in a class handle seems to be unable to access properties of the class. Here is an example:


classdef myClass < handle
  properties
    d;
  end
  methods
    function h=myClass(d)
      h.d=d;
    end
    function delete(h)
      disp('Destructor called');
      x=h.d;
      disp(x);
    end
  end
end


This produces the following error:

>> h=myClass(4); clear h
Destructor called
warning: error caught while executing handle class delete method:
value on right hand side of assignment is undefined


So the delete method cannot access the property 'd'.

However, calling the delete method directly, it works fine:

>> h=myClass(4); delete(h)
Destructor called
 4


This behavior seems to be new in Octave 4.4.0, because previously the destructor was not called at all, see bug #46497. I tested this on octave-4.4.0-w64 on Windows.

Is it possible that the clear command deletes the properties of the class before calling the 'delete' method?

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #44147:  diffs.txt added by jwe (4KiB - text/plain)

 

Digest:
   bug dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by amro_octave
  • -email is unavailable- added by crobar (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-02-21 mtmiller StatusPostponed Fixed
        Open/ClosedOpen Closed
        Summaryhandle class: delete destructor method cannot access class properties classdef: delete destructor method cannot access class properties
    2018-10-10 amro_octave Carbon-Copy- Added amro_octave
    2018-05-22 mtmiller Dependencies- Depends on bugs #46497
    2018-05-15 jwe StatusReady For Test Postponed
    2018-05-14 jwe StatusIn Progress Ready For Test
    2018-05-13 jwe StatusReady For Test In Progress
    2018-05-13 jwe Attached File- Added diffs.txt, #44147
    2018-05-11 jwe StatusConfirmed Ready For Test
    2018-05-09 mtmiller StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code