bugGNU Octave - Bugs: bug #53844, handle class: delete destructor...

 
 

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

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

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

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>
Project Administrator
Tue 15 May 2018 02:15:15 AM UTC, comment #17:

I see a stack trace that begins with:

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>
Project 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:

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>
Project Administrator
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>
Project Administrator
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>
Project 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

And the following examples

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>
Project Administrator
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>
Project Administrator
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>
Project 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>
Project 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

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

Mike Miller <mtmiller>
Project Administrator
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.

Rik <rik5>
Project 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>
Project Administrator
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

Now the following code at the interpreter prompt shows the problem

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>
Project Administrator
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>
Project Administrator
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>
Project 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>
Project Administrator
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>
Project 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:

This produces the following error:

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

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

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 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)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    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.3