bugGNU Octave - Bugs: bug #54600, edit mfile with an error crashes...

 
 

bug #54600: edit mfile with an error crashes octave

Submitter:  Marshall <marsian>
Submitted:  Fri 31 Aug 2018 01:07:36 AM UTC
   
 
Category:  GUI Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  None
Originator Name:  Marsian Open/Closed:  * Closed
Release:  * 4.4.1 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 24 Sep 2018 11:10:07 PM UTC, comment #16: 

This works for me: No crashes.

Marshall <marsian>
Sat 22 Sep 2018 07:52:31 PM UTC, comment #15: 

I have pushed changeset
http://hg.savannah.gnu.org/hgweb/octave/rev/cb440b43a236
which implements the usage of Fexist and Ffile_in_loadpath instead of F__which__

Torsten Lilge <ttl>
Group Member
Fri 21 Sep 2018 06:19:38 PM UTC, comment #14: 

Regardless of this bug, yes I think you should file a separate bug about 'which' returning an error when octave fails to parse the m-file.

Mike Miller <mtmiller>
Group Member
Fri 21 Sep 2018 05:16:29 PM UTC, comment #13: 

It is at least an incompatibility with Matlab as per the example in comment #9.

If F__which__ didn't throw an error, would it still crash calling it from the GUI thread?

Markus Mützel <mmuetzel>
Group administrator
Fri 21 Sep 2018 04:27:45 PM UTC, comment #12: 

I am not sure whether the behavior of F__which__ can really be seen as a bug. To my understanding, F.... functions from the octave API are intended to be called from the octave thread and calling from the gui thread might lead to unexpected behavior (please correct me if I am wrong here).

Torsten Lilge <ttl>
Group Member
Fri 21 Sep 2018 04:20:17 PM UTC, comment #11: 

Maybe the best would be to use a try-catch block to avoid the crash until F__which__ is fixed.

Should I open a separate bug report for the problem with "which"?

Markus Mützel <mmuetzel>
Group administrator
Fri 21 Sep 2018 03:35:11 PM UTC, comment #10: 

Currently, F__which__ only crashes for files with syntax error when called from the gui thread. The "edit" function uses "exist" and "file_in_path" in order to find the editable m-file of a function. Therefore, I am currently looking for the following possible solutions:

  • Try to use Fexist together with Ffile_in_loadpath as the "edit" command does
  • Call F__which__ via the octave thread (no crash, only an error message about the syntax error) and try to pass the results to the gui thread
  • Use a try-block


Torsten Lilge <ttl>
Group Member
Fri 21 Sep 2018 07:41:41 AM UTC, comment #9: 

In Matlab, "which" on a function that isn't executable doesn't throw an error:

>> type test_wrong

function 5 = test()

end
>> test_wrong
Error: File: test_wrong.m Line: 1 Column: 10
Unexpected MATLAB expression.
>> which test_wrong
D:\temp\test_wrong.m
>>


So maybe "F__which__" should be changed as Pantxo suggested.

Markus Mützel <mmuetzel>
Group administrator
Fri 21 Sep 2018 07:26:31 AM UTC, comment #8: 

I think it would be interesting to know if in Matlab "which fcn" leads to an error when "fcn" happens to not be executable. Personally, what I want when I use "which fcn" is Octave to answer the question "In which file will you search for the definition of fcn?". So I would be inclined to change F__which__ behavior, which would fix this bug.

@Torsten: About comment #4, if you really want an error that doesn't crash the GUI, you can use a try/catch and rethrow the error as we do when printing:


try
  {
    // Make sure we have a valid current context
    if (! begin_rendering ())
      error ("print: no valid OpenGL offscreen context");

    octave::gl2ps_print (figObj, file_cmd.toStdString (),
                                  term.toStdString ());
  }
catch (octave::execution_exception& e)
  {
    octave_link::post_exception (std::current_exception ());
    end_rendering ();
  }


Pantxo Diribarne <pantxo>
Group Member
Thu 20 Sep 2018 04:50:33 PM UTC, comment #7: 

One option would be to just launch the command


edit this_function


with the following drawbacks:

  • not responsive if the worker thread is busy
  • does not find the file if not in the current path but in the directory of the currently edited file or in a "private" subdirectory


However, the "edit" command actually opens a file with syntax error but still detects built in functions. In "edit", the method "exist" is used. I will have a look whether this can be used here, too.


Torsten Lilge <ttl>
Group Member
Thu 20 Sep 2018 04:07:07 PM UTC, comment #6: 

In any case, the short term fix for this looks to be doing "something" to avoid the crash. That still means that when the user right-clicks and chooses "Edit this_function", it will fail. Is there some other function we can call that can find the file without relying on the contents of the file being valid?

Mike Miller <mtmiller>
Group Member
Thu 20 Sep 2018 04:00:47 PM UTC, comment #5: 

I see, you think the crash may be caused by a multi-threading issue in the symbol table routines. It seems that this kind of thing is coming up more and more, maybe we need to spend some effort making the symbol and path interpreter functions thread-safe.

I originally read this as just an uncaught exception causing a crash, since all errors are exceptions now, but I haven't looked at the stack trace.

Mike Miller <mtmiller>
Group Member
Thu 20 Sep 2018 03:57:10 PM UTC, comment #4: 

I am going to test this approach. Although it may fix the bug, it does not really fix the cause for the crash.

As far as I have tested, the crash happens because F__which__ is called from the gui thread and not from the worker thread. So I am currently working on a method for calling F__which__ from the worker thread and passing the results to the gui thread. However, this might block the gui thread if the worker thread is already processing a longer computation.


Torsten Lilge <ttl>
Group Member
Thu 20 Sep 2018 01:36:18 AM UTC, comment #3: 

It looks to me like main_window::handle_edit_mfile_request simply needs a try-catch when it calls '__which__' to catch the exception thrown by the function in case of a parse error or other error.

Mike Miller <mtmiller>
Group Member
Fri 14 Sep 2018 08:04:35 PM UTC, comment #2: 

For determining the file to edit, F__which__ is used and it seems that


symtab.find_function (name)


crashes when called with a file containing a parsing error.

Torsten Lilge <ttl>
Group Member
Fri 31 Aug 2018 05:03:17 AM UTC, comment #1: 

Confirmed on Linux, so chnageing OS to "Any".

Trying to edit files with parsing errors via the editor's (context-)menu crashes octave.

Torsten Lilge <ttl>
Group Member
Fri 31 Aug 2018 01:07:36 AM UTC, original submission:  

I've had this happen on several occasions now, where I create a new function, run it, get an error, then try to open it via the keyboard edit file shortcut (ctl E is the default, but I have changed it...), and then octave immediately crashes. It has been very repeatable.
The shortcut works on files without an error. Typing "edit file..." works without crashing.

Here's an example:
save a file editCrash1.m with

function [a] editCrash1()
    a=1;
endfunction

in the editor type editCrash1 somewhere, click on it and type the edit keyboard shortcut (e.g. ctl E). Octave crashes.

2nd example:
save a file editCrash2.m with

function editCrash2()
    a=;
endfunction

in the editor type editCrash2 somewhere, click on it and type the edit keyboard shortcut (e.g. ctl E). Octave crashes.

Marshall <marsian>

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by pantxo (Posted a comment)
  • -email is unavailable- added by ttl (Posted a comment)
  • -email is unavailable- added by marsian (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 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-09-25 ttl StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2018-09-22 ttl StatusConfirmed Ready For Test
    2018-09-20 mtmiller CategoryNone GUI
    2018-08-31 ttl StatusNone Confirmed
        Operating SystemMicrosoft Windows Any

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code