bugGNU Octave - Bugs: bug #40319, deleting script file in a...

 
 

bug #40319: deleting script file in a function/script doesn't update the symbol table

Submitter:  Pantxo Diribarne <pantxo>
Submitted:  Sun 20 Oct 2013 09:29:09 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  In Progress Assigned to:  None
Originator Name:  Open/Closed:  * Open
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 03 Jul 2016 11:45:13 AM UTC, comment #10: 

Retagging as "in progress", because the performance penalty of the rehashing has not yet been evaluated, and probably won't be before 4.2.0.

Lachlan Andrew <lachlan>
Thu 02 Jun 2016 12:59:13 AM UTC, comment #9: 

Pantxo, your patch looks compact.  However, the rehash may be quite slow, and it quite likely that user code would call "exist" often to see if a variable exists (without specifying that), which doesn't need the rehash.

A more invasive patch would be to store a flag recording whether any file has been created or destroyed since the last command prompt, and only rehash if that is the case.

That would require checking for all Octave functions that create or delete files, and possibly also setting the flag if an OCT/MEX file has been run, since they may create files directly.

I'm not sure if the increased complexity is worth it for the increase in speed.

Lachlan Andrew <lachlan>
Sun 27 Apr 2014 07:26:01 PM UTC, comment #8: 

I ran the attached testdelete2.m (besides base.m) in Matlab in order to see how Matlab handles the case of file deletion/modification.

Basically there are too functions "foo.m" and "foo/foo.m" in the loadpath (they print their mfilename). The first in the path ("foo.m") is deleted or modified:


 * Deleting 'foo.m' before 'which ('foo')' returns
/home/pantxo/media/dev/octavewrk/bugdelete/foo/foo.m

 * Deleting 'foo.m' before 'exist ('foo.m', 'file')' returns
ans =
     2

 * Deleting 'foo.m' before 'run ('foo')' returns
ans =
/home/pantxo/media/dev/octavewrk/bugdelete/foo/foo

 * Deleting 'foo.m' before bare 'foo' returns
Previously accessible file "/home/pantxo/media/dev/octavewrk/bugdelete/foo.m" is now inaccessible.

 * Modifying 'foo.m' before bare 'foo' returns
ans =
/home/pantxo/media/dev/octavewrk/bugdelete/foo
modified


It shows that:

  • matlab rehashes prior to search in "run", "which" or "exist". * if a file "foo.m" has been deleted since the last rehash, trying to run it using bare "foo" leads to an error (as in octave) even if another "foo.m" exits in the path.
  • I was wrong in comment #7 about octave behavior in case of file modification: the modified file is parsed in octave as in matlab



The attached simple patch brings octave in line with matlab except that the latters's message (in case of bare call "foo") is more explicit about the origin of the error. 


(file #31260, file #31261, file #31262)

Pantxo Diribarne <pantxo>
Group Member
Fri 14 Feb 2014 01:44:09 PM UTC, comment #7: 

Sorry for the late reply. I agree with you that there are many ways to make a function file disappear and we should not try to manage all of them individually.

The reason why the example works in the terminal and not in a script, is that the path is updated before parsing each input from the prompt (in octave_base_reader::octave_gets from input.cc).

For exist and which functions it seems reasonable to update the path before anything is done.

As it is not reasonable to update the path after each line of a script/function, a test is run (out_of_date_check) from various symbol_table::fcn_info::fcn_info_rep::find_xx functions.

Unfortunatly, the test only checks that the last time the function file was checked is ulterior to last prompt and last dir change.

As a consequence, not only deleted files, but also modified are affected : let's say I have a script toto.m that modifies a second script toto2.m and then runs it. The old version of toto2.m will be run because according to the criteria in out_of_date_check, it is up to date and doesn't even have to be parsed again.
 
The test should also check that the last time the file was modified is anterior to both last prompt and last dir change. Not sure this may be done fast enough (with stat?).



Pantxo Diribarne <pantxo>
Group Member
Thu 24 Oct 2013 03:40:54 AM UTC, comment #6: 

Because delete may not always be used to delete script files? It seems a little hackish to me to reload the path in a function that may or may not add or remove a function that's in the symbol table. What about unlink("script.m")? What about system("rm -f script.m")? Or what if script is never called again after the delete, why bother reloading the path?

It seems better to try to reload the path and look up again on demand when a symbol is actually used and can't be found.

Mike Miller <mtmiller>
Group Member
Wed 23 Oct 2013 01:35:26 PM UTC, comment #5: 

Why not rehash in delete.m if we have deleted a script file that were in the path. Then delete.m is responsible for letting the symbol table in an up to date state.


Pantxo Diribarne <pantxo>
Group Member
Tue 22 Oct 2013 01:10:12 PM UTC, comment #4: 

Here's how far I've dug into this:

The exist function ends up calling the static symbol_table::find function. Further down in the symbol_table class are a couple of calls to load_path::update(), which is what rehash does. It looks like there is some logic in place to attempt to reload the path if a find fails, but I haven't traced the symbol_table class enough yet to know under what conditions it's done.

Mike Miller <mtmiller>
Group Member
Tue 22 Oct 2013 07:26:31 AM UTC, comment #3: 

I didn't know the rehash command. That's exactly what I think should be done right after deleting a file which is in the path.

I see no good reason why that script should work in the command line (if I copy/paste in octave prompt I obtain the good results immediatly, no need to rehash) and not from a function/script file.


Pantxo Diribarne <pantxo>
Group Member
Tue 22 Oct 2013 01:24:39 AM UTC, comment #2: 

I'm not sure, but perhaps this is expected behavior? For example, after the testdelete script returns, exist returns 2. Also if I add


rehash;
retval = exist ("scripta.m", "file")


to the end of testdelete, I first get retval = 0, then retval = 2. Is it perhaps expected that the load path is only automatically refreshed when Octave is interactive and it returns to the prompt?

Mike Miller <mtmiller>
Group Member
Sun 20 Oct 2013 09:32:32 PM UTC, comment #1: 

Sorry, I forgot to mention that the first part was meant to be run from a script (attached).



(file #29432)

Pantxo Diribarne <pantxo>
Group Member
Sun 20 Oct 2013 09:29:09 PM UTC, original submission:  

If I delete a script file in a script or a function, I have to go back to prompt to have the symbol table updated. The following test shows the bug:


cd (tempdir ());

## create two files
id = fopen ("scripta.m", "w+");
fclose (id);
mkdir ("foo");
id = fopen ("foo/scripta.m", "w+");
fclose (id);

addpath (fullfile (tempdir, "foo"));

## delete the first one in the path
file = which ("scripta");
delete (file);
retval = exist ("scripta.m", "file")


retval should be 2 and I see 0 instead. Back in the octave prompt, evaluating  the last line gives the good result:



>> exist ("scripta.m", "file")
ans =  2


Pantxo Diribarne <pantxo>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #31260:  delete_exist.patch added by pantxo (1KiB - application/x-download)
file #31261:  base.m added by pantxo (23B - text/x-objcsrc)
file #31262:  testdelete2.m added by pantxo (1KiB - text/x-objcsrc)
file #29432:  testdelete.m added by pantxo (305B - text/x-objcsrc)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by lachlan (Updated the item)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by pantxo (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-07-03 lachlan StatusPatch Submitted In Progress
    2016-06-02 lachlan StatusNeed Info Patch Submitted
    2014-04-27 pantxo Attached File- Added delete_exist.patch, #31260
        Attached File- Added base.m, #31261
        Attached File- Added testdelete2.m, #31262
    2013-10-22 mtmiller CategoryNone Interpreter
        StatusNone Need Info
    2013-10-20 pantxo Attached File- Added testdelete.m, #29432

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code