leveldb::DestroyDB fails on Windows

[GoogleCodeExporter]

I'm using Chromium's Env for Windows, but this should apply to any Env 
implementation on that OS.

In db_impl.cc's DestroyDB:

const std::string lockname = LockFileName(dbname);

This is defined to be dbname + "/LOCK" in filename.cc

Later on:

      if (ParseFileName(filenames[i], &number, &type) &&
          filenames[i] != lockname) {  // Lock file will be deleted at end                                              
        Status del = env->DeleteFile(dbname + "/" + filenames[i]);
        if (result.ok() && !del.ok()) {
          result = del;
    }
      }

filenames[i] contains just "LOCK", but lockname will be the full path, so the 
expression filenames[i] != lockname will always be false. Therefore, DeleteFile 
will always be called, and possibly fail. If the Env can't delete the LOCK file 
(e.g. Windows file semantics), the DestroyDB will fail.

Two possible fixes:

     for (size_t i = 0; i < filenames.size(); i++) {
-      if (ParseFileName(filenames[i], &number, &type) &&
-          filenames[i] != lockname) {  // Lock file will be deleted at end
-        Status del = env->DeleteFile(dbname + "/" + filenames[i]);
-        if (result.ok() && !del.ok()) {
-          result = del;
+      if (ParseFileName(filenames[i], &number, &type)) {
+        const std::string filepath = dbname + "/" + filenames[i];
+        if (filepath != lockname) {  // Lock file will be deleted at end
+          Status del = env->DeleteFile(filepath);
+          if (result.ok() && !del.ok()) {
+            result = del;
+          }
         }

Or what I assume is preferable:

       if (ParseFileName(filenames[i], &number, &type) &&
-          filenames[i] != lockname) {  // Lock file will be deleted at end
+          type != kDBLockFile) { // Lock file will be deleted at end
         Status del = env->DeleteFile(dbname + "/" + filenames[i]);

Original issue reported on code.google.com by jsbell@chromium.org on 8 Mar 2012 at 7:51

[GoogleCodeExporter]

Thanks.  Will fix soon.

Original comment by san...@google.com on 9 Mar 2012 at 12:33

• Changed state: Accepted

[GoogleCodeExporter]

This looks like a duplicate of Issue #18, which was resolved fixed but the fix 
was apparently never verified.

http://code.google.com/p/leveldb/issues/detail?id=18

Original comment by jsbell@chromium.org on 9 Mar 2012 at 3:44

[GoogleCodeExporter]

Fixed.  Please verify.

Original comment by san...@google.com on 9 Mar 2012 at 3:55

• Changed state: Fixed

[GoogleCodeExporter]

Verified - fix works as expected.

Original comment by jsb...@google.com on 9 Mar 2012 at 7:00

[GoogleCodeExporter]

Original comment by san...@google.com on 9 Mar 2012 at 7:02

• Changed state: Verified