bugGNU Octave - Bugs: bug #56618, In dir.m, if internal call to...

 
 

bug #56618: In dir.m, if internal call to lstat fails, an error is thrown unnecessarily

Submitted by:  Richard <crobar>
Submitted on:  Fri 12 Jul 2019 01:38:29 AM UTC  
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  Richard Crozier Open/Closed:  Closed
Release:  dev Operating System:  Any

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)

Mon 15 Jul 2019 10:43:10 PM UTC, comment #7: 

Okay, I am going to close the bug presumtively since it passed my tests.  But, post a comment back to this bug report and I will re-open if necessary.

Rik <rik5>
Project Administrator
Mon 15 Jul 2019 10:17:34 PM UTC, comment #6: 

I will try and test as soon as I can, but it may take a few weeks.

Richard <crobar>
Fri 12 Jul 2019 04:14:14 PM UTC, comment #5: 

I think you're right that there is a race condition between when we get the file name, and when we check its properties.

For the issue with stat, I think the best course is to change the code too look for nf > 0 before proceeding.

The issue with lstat is a little trickier.  I chose to remove the file from the list if lstat fails.

Try the development branch after this cset (https://hg.savannah.gnu.org/hgweb/octave/rev/733431da9742).

Marking as Ready For Test.

Rik <rik5>
Project Administrator
Fri 12 Jul 2019 11:33:53 AM UTC, comment #4: 

Incidentally, there is a similar issue when there is a failed call  to 'stat' above:

  if (nf == 1)
    fn = flst{1};
    [st, err, msg] = stat (fn);
    if (err < 0)
      warning ("dir: 'stat (%s)' failed: %s", fn, msg);
      nf = 0;
    elseif (S_ISDIR (st.mode))
      flst = readdir (flst{1});
      nf = numel (flst);
      flst = strcat ([fn filesep], flst);
    endif
  endif

if stat fails, nf is set to zero, then later we try to do this:

info(nf,1).name = "";  # pre-declare size of struct array

which fails with an indexing error as nf is zero. We should probably not enter the following if statement if nf == 0

Richard <crobar>
Fri 12 Jul 2019 10:42:42 AM UTC, comment #3: 

Well, personally, this would break my code, which checks for empty fields in the structure. I'd have to wrap it in a try-catch instead. Other users might be affected (but perhaps not many).

It may also break compatibility with matlab, I don't know what it does for sure, I don't know if the issue has happened to me on matlab, or if they are just faster at getting the info. In my case I am using a modified version of the multicore package (the original is no longer maintained on Octave Forge) which communicates between workers using files on a common file system. I think the problem for me is that the file is being deleted in between it being found and the call to lstat. This possibility is accounted for in the multicore package and the file is ignored if  certain fields are empty in the structure.

If the call to lstat could happen sooner after getting the file list it might help.

Another option is to check again if the file actually still exists in the event that lstat fails, and remove it from the list if it doesn't.

Richard <crobar>
Fri 12 Jul 2019 02:50:12 AM UTC, comment #2: 

What about changing the call to warning() to a call to error()?  If lstat has failed that seems pretty serious.

Rik <rik5>
Project Administrator
Fri 12 Jul 2019 02:02:10 AM UTC, comment #1: 

It looks like the following commit introduced the regression:

http://hg.savannah.gnu.org/hgweb/octave/file/21fc54e4bb7b/scripts/miscellaneous/dir.m

Richard <crobar>
Fri 12 Jul 2019 01:38:29 AM UTC, original submission:  

This is a very hard to reproduce bug, but I think the problem should be clear enough.

When calling dir.m, internally lstat is called to get info on files. Early in dir.m this info structure is initialized like so:

  info = struct (zeros (0, 1),
           {"name", "folder" "date", "bytes", "isdir", "datenum", "statinfo"});

There is a call to lstat at line 124. If this is successful, this structure is populated, including the info.datenum field.

Now, if the call to lstat fails at line 124, there is an if statement which tests for an error, and if there was an error, a warning is issued and the structure is never populated, so info.datenum is still empty.

After this, however, outside the if statement there is the following:

## A lot of gymnastics in order to call datenum just once.  2x speed up.
    dvec = [info.datenum]([[1:6:end]', [2:6:end]', [3:6:end]', ...
                           [4:6:end]', [5:6:end]', [6:6:end]']);
    dnum = datenum (dvec);
    ctmp = mat2cell (dnum, ones (nf,1), 1);

So the still empty info.datenum field is attempted to be used, but results in dvec being empty, dnum being empty, and ultimately you get an error from mat2cell like so:

warning: called from
    dir at line 126 column 9
    setfilesemaphore at line 94 column 23
    loadslaveparams at line 69 column 9
    startmulticoreslave2 at line 273 column 90
error: mat2cell: mismatch on dimension 1 (0 != 1)
error: called from
    dir at line 159 column 10
    setfilesemaphore at line 94 column 23
    loadslaveparams at line 69 column 9
    startmulticoreslave2 at line 273 column 90

At the very least this is a bug due to incompatibility with Matlab, but I think this is a straight out bug!

I'm pretty sure this used to work fine, so I have labelled it as a regression. I suspect someone has done the gymnastics in the interim. Probably this could just be reverted to the pre 2X speedup version mentioned in the comment.

Richard <crobar>

 

(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 rik5 (Posted a comment)
  • -email is unavailable- added by crobar (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.

     

     

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-07-15 rik5 StatusReady For Test => Fixed
        Open/ClosedOpen => Closed
    2019-07-12 rik5 StatusNeed Info => Ready For Test
        Release5.1.0 => dev
        SummaryIn dir.m, if internal call to lstat falis, an error is thrown unnecessarily => In dir.m, if internal call to lstat fails, an error is thrown unnecessarily
    2019-07-12 rik5 StatusNone => Need Info

    Back to the top


    Powered by Savane 3.5