Bug #45402 Win32AsyncFile::rmrfReq can run an infinite loop

Description:
sometimes with ndbd --initial, Win32AsyncFile::rmrfReq runs an infinite loop.  the code in Win32AsyncFile::rmrfReq is not pretty and a logical re-write should fix the problem.  

How to repeat:
.

Suggested fix:
i'm sure there's example code on the web to remove a directory tree.

I suggest moving the "rm -rf" to AsyncFile so that same code runs on Windows as on other platforms. Should be no limit in using standard posix function for this task.

there is no AsyncFile::rmrfReq implementation.

functions missing from windows vc runtime that are used in PosixAsyncFile:

  opendir(path)
  readdir(dirp)
  remove(path)
  closedir(dirp)

i'll add AsyncFile::opendir() etc. methods and implement them

the problem with Win32AsyncFile::rmrfReq is that
it can't remove empty directories.

using DirIterator -- have posted a patch to dev-ndb

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/76668

2936 jack andrews	2009-06-19
      Bug #45402  	Win32AsyncFile::rmrfReq can run an infinite loop
       . worked on supporting code
       . with test case (that does a rm -rf)

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/77248

2936 jack andrews	2009-06-25
      Bug #45402   Win32AsyncFile::rmrfReq can run an infinite loop
        . supporting code with test
        . test code does a rm -rf

I still prefer the old one, where you process one file at a time. It currently has a "next_file" iterator. It should be fairly trivial to extend that with  a "next_entry" iterator that gives you next file _or_ dir.

bzr mv storage/ndb/src/mgmsrv/DirIterator.hpp storage/ndb/include/util/
bzr mv storage/ndb/src/mgmsrv/DirIterator.cpp storage/ndb/include/util/

Fix Makefile.am's

Implement new function with the following prototype.... To get it as portable as possible filter away the . and .. behind the scenes.

=== modified file 'storage/ndb/src/mgmsrv/DirIterator.hpp'
--- storage/ndb/src/mgmsrv/DirIterator.hpp    2008-10-24 12:41:10 +0000
+++ storage/ndb/src/mgmsrv/DirIterator.hpp    2009-06-15 18:36:48 +0000
@@ -24,6 +24,7 @@ public:

   int open(const char* path);
   const char* next_file(void);
+  const char* next_entry(bool& is_dir);
 };

> I still prefer the old one, where you process one file at a time. 

the patch processes one file at a time like the old one.

> It currently has a "next_file" iterator. It should be fairly trivial 
> to extend that with  a "next_entry" iterator that gives you next 
> file _or_ dir.

yes, it does that.

> bzr mv storage/ndb/src/mgmsrv/DirIterator.hpp storage/ndb/include/util/
> bzr mv storage/ndb/src/mgmsrv/DirIterator.cpp storage/ndb/include/util/

i will do that.

> Fix Makefile.am's

i think they work

> Implement new function with the following prototype.... 
> +  const char* next_entry(bool& is_dir);

that is exactly what is in the patch

> To get it as portable as possible
> filter away the . and .. behind the scenes.

it does that.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/77438

2936 jack andrews	2009-06-29
      Bug #45402  	Win32AsyncFile::rmrfReq can run an infinite loop
        - the first step:
           . move mgmsrv/DirIterator.?pp to util
           . alter make files to build ndbgeneral with DirIterator
        - more to come.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/77472

2937 jack andrews	2009-06-29
      Bug #45402  	Win32AsyncFile::rmrfReq can run an infinite loop
        - implemented and tested win32 implementation
        - need to check interface is OK

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/77643

2937 jack andrews	2009-07-01
      Bug #45402        Win32AsyncFile::rmrfReq can run an infinite loop
      
              - implemented and tested win32 implementation

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/77692

2938 jack andrews	2009-07-01
      Bug #45402  	Win32AsyncFile::rmrfReq can run an infinite loop
         - ported to *n?x 
         - added test for symlink

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/77817

2939 jack andrews	2009-07-02
      Bug #45402  	Win32AsyncFile::rmrfReq can run an infinite loop
      
        - integrated with Win32AsyncFile
        - casual testing shows this is a fix for this bug

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/77898

2940 jack andrews	2009-07-03
      Bug #45402  	Win32AsyncFile::rmrfReq can run an infinite loop
        - added more remove() functions because AsyncFile does more
          than just a rmrf on a directory. it 
             . deletes files 
             . the contents of the dir (like rm -rf dir/*)

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/77911

2941 jack andrews	2009-07-03
      Bug #45402  	Win32AsyncFile::rmrfReq can run an infinite loop
        - added more functions to handle rmrf logic in AsyncFile.
        - removed rmrf from Posix and Win32 AsyncFile
        - integrated with AsyncFile.hpp

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/77914

2942 jack andrews	2009-07-03
      Bug #45402  	Win32AsyncFile::rmrfReq can run an infinite loop
        - fixed a bug:  old behaviour is to return without error if 
                        the path doesn't exist.
          made it work like the original

Looked at the patches, have a few comments but overall it looks ok. Could you send a complete patch or the whole files?

bzr diff -r-8..

Attachment: bdiff (application/octet-stream, text), 18.80 KiB.

the changed files

Attachment: files.tgz (application/octet-stream, text), 23.22 KiB.

this bug is redundant as magnus blaudd has written a different one.

=== modified file 'storage/ndb/src/kernel/blocks/ndbfs/Win32AsyncFile.cpp'
--- storage/ndb/src/kernel/blocks/ndbfs/Win32AsyncFile.cpp      2009-05-28 18:54
:28 +0000
+++ storage/ndb/src/kernel/blocks/ndbfs/Win32AsyncFile.cpp      2009-09-14 09:07
:16 +0000
@@ -402,7 +402,7 @@
     {
       int len = strlen(path);
       strcat(path, ffd.cFileName);
-      if(DeleteFile(path))
+      if(DeleteFile(path) != 0 || RemoveDirectory(path) != 0)
       {
         path[len] = 0;
        continue;

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/83171

Pushed to 7.0 and 7.1

Documented bugfix in the NDB-7.0.8 changelog as follows:

        On Windows, ndbd --initial could hang in an endless loop while
        attempting to remove directories.

Closed.