bugGNU Octave - Bugs: bug #52024, Memory issue using File->Close...

 
 

bug #52024: Memory issue using File->Close from FLTK figure window

Submitted by:  Rik <rik5>
Submitted on:  Thu 14 Sep 2017 07:59:25 PM UTC  
 
Category: Plotting with OpenGLSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Segfault, Bus Error, etc.
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating System: Any

Add a New Comment(Rich Markup)
   

You are not logged in

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

 

Thu 21 Sep 2017 06:44:50 PM UTC, comment #5:

Tested the cset and it works just fine. Marking as fixed and closing report.

Rik <rik5>
Project Administrator
Thu 21 Sep 2017 05:57:36 PM UTC, comment #4:

Casting to intptr_t seems OK to me, but I decided to hide that in a custom class and use insert instead of emplace and add a member function callback to the unwind-protect stack instead of attempting to restore the value of the std::set variable.

I pushed this modified version of your changeset:

http://hg.savannah.gnu.org/hgweb/octave/rev/3f036b46a130

It seems to work for me but I'll wait for confirmation and any additional comments before closing.

John W. Eaton <jwe>
Project Administrator
Thu 21 Sep 2017 02:26:34 PM UTC, comment #3:

In an unwind_protect object, I don't think there is a way to know if the pointer to a variable is pointing to a freed heap memory or not. If I am not wrong on that point, it is not safe to use unwind_protect objects to restore variables that we can't be sure will still be allocated at restore time.

I attached a tentative patch that calls for criticism since (i) I am not sure it is good practice to cast pointers to int, (ii) I don't know how to check things still work as expected, i.e. a callback can't be executed recursively.

(file #41854)

Pantxo Diribarne <pantxo>
Project Member
Wed 20 Sep 2017 02:45:21 PM UTC, comment #2:

If I do the following change I don't see the heap-use-after-free anymore:

The issue here is that "executing" is an attribute of the callback_property object which will be destroyed by the callback execution and will thus be already freed when the unwind_protect object tries to restore its value.

Pantxo Diribarne <pantxo>
Project Member
Tue 19 Sep 2017 06:12:21 PM UTC, comment #1:

The issue is still not completely understood, but for a new FLTK figure the function _add_default_menu_.m is called. This creates the uimenu object for the figure. One of the menu entries is File->Close and the associated callback is the pure piece of Octave code "close (gcf)".

The issue seems to be that FLTK detects the menu click, and then the uimenu object attempts to run the callback code in an unwind/protect block. The callback code closes the figure which deletes the uimenu object. When control returns from the callback, the unwind protect block now has a non-existent uimenu object. That's all I can tell so far. I don't have a good way of solving this just yet.

Rik <rik5>
Project Administrator
Thu 14 Sep 2017 07:59:25 PM UTC, original submission:

Creating a figure and then closing it with "File->Close" creates a use-after-free error. If the figure is closed from the command line using "close" then there is no such problem.

Sample code:

The backtrace is

Rik <rik5>
Project Administrator

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #41854:  safe_callback_exec.patch added by pantxo (3KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by pantxo (Posted a comment)
  • -unavailable- added by rik5 (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to 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 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 21 Sep 2017 06:44:50 PM UTCrik5StatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Thu 21 Sep 2017 05:57:36 PM UTCjweStatusConfirmed=>Ready For Test
    Thu 21 Sep 2017 02:26:34 PM UTCpantxoAttached File-=>Added safe_callback_exec.patch, #41854
    Wed 20 Sep 2017 02:45:28 PM UTCpantxoStatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup1