Bug #45402 Win32AsyncFile::rmrfReq can run an infinite loop
Submitted: 9 Jun 2009 14:20 Modified: 16 Sep 2009 13:58
Reporter: jack andrews Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Cluster: Cluster (NDB) storage engine Severity:S3 (Non-critical)
Version:mysql-5.1-telco-7.0 OS:Windows (vista)
Assigned to: Magnus Blåudd CPU Architecture:Any

[9 Jun 2009 14:20] jack andrews
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.
[10 Jun 2009 8:15] Magnus Blåudd
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.
[10 Jun 2009 10:10] jack andrews
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
[10 Jun 2009 10:11] jack andrews
the problem with Win32AsyncFile::rmrfReq is that
it can't remove empty directories.
[18 Jun 2009 13:31] jack andrews
using DirIterator -- have posted a patch to dev-ndb
[19 Jun 2009 11:18] Bugs System
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)
[25 Jun 2009 19:18] Bugs System
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
[26 Jun 2009 8:04] Magnus Blåudd
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);
 };
[26 Jun 2009 9:11] jack andrews
> 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.
[29 Jun 2009 10:18] Bugs System
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.
[29 Jun 2009 14:04] Bugs System
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
[1 Jul 2009 11:04] Bugs System
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
[1 Jul 2009 18:40] Bugs System
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
[2 Jul 2009 18:41] Bugs System
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
[3 Jul 2009 12:59] Bugs System
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/*)
[3 Jul 2009 14:34] Bugs System
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
[3 Jul 2009 14:48] Bugs System
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
[7 Jul 2009 15:06] Magnus Blåudd
Looked at the patches, have a few comments but overall it looks ok. Could you send a complete patch or the whole files?
[7 Jul 2009 15:28] jack andrews
bzr diff -r-8..

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

[7 Jul 2009 15:32] jack andrews
the changed files

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

[11 Sep 2009 7:16] jack andrews
this bug is redundant as magnus blaudd has written a different one.
[14 Sep 2009 9:12] Magnus Blåudd
=== 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;
[14 Sep 2009 12:43] Bugs System
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
[14 Sep 2009 14:59] Magnus Blåudd
Pushed to 7.0 and 7.1
[16 Sep 2009 13:58] Jon Stephens
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.