ntdb: don't munmap the database on every close.

Since we can have multiple openers, we should leave the mmap in place
for the other openers to use.  Enhance the test to check the bug (it
still works, because without mmap we fall back to read/write, but
performance would be terrible!).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/lib/ntdb/open.c b/lib/ntdb/open.c
index 9de9e9b48cc992873933cd1ce248b3520a85dc20..56c97afe4331bb1db9a72f3c0df9724ebaf2bcbd 100644 (file)
--- a/lib/ntdb/open.c
+++ b/lib/ntdb/open.c
@@ -868,19 +868,19 @@ _PUBLIC_ int ntdb_close(struct ntdb_context *ntdb)
                ntdb_transaction_cancel(ntdb);
        }
 
-       if (ntdb->file->map_ptr) {
-               if (ntdb->flags & NTDB_INTERNAL)
-                       ntdb->free_fn(ntdb->file->map_ptr, ntdb->alloc_data);
-               else
-                       ntdb_munmap(ntdb->file);
-       }
-       if (ntdb->file) {
-               ntdb_lock_cleanup(ntdb);
-               if (--ntdb->file->refcnt == 0) {
-                       ret = close(ntdb->file->fd);
-                       ntdb->free_fn(ntdb->file->lockrecs, ntdb->alloc_data);
-                       ntdb->free_fn(ntdb->file, ntdb->alloc_data);
+       ntdb_lock_cleanup(ntdb);
+       if (--ntdb->file->refcnt == 0) {
+               if (ntdb->file->map_ptr) {
+                       if (ntdb->flags & NTDB_INTERNAL) {
+                               ntdb->free_fn(ntdb->file->map_ptr,
+                                             ntdb->alloc_data);
+                       } else {
+                               ntdb_munmap(ntdb->file);
+                       }
                }
+               ret = close(ntdb->file->fd);
+               ntdb->free_fn(ntdb->file->lockrecs, ntdb->alloc_data);
+               ntdb->free_fn(ntdb->file, ntdb->alloc_data);
        }
 
        /* Remove from tdbs list */

diff --git a/lib/ntdb/test/api-open-multiple-times.c b/lib/ntdb/test/api-open-multiple-times.c
index 217732d227ba2d8622df26ac5c36a340160b56ab..8663b8a24c4f94191ab4ec2c3bb691df9c415319 100644 (file)
--- a/lib/ntdb/test/api-open-multiple-times.c
+++ b/lib/ntdb/test/api-open-multiple-times.c
@@ -6,6 +6,7 @@
 #include <fcntl.h>
 #include <stdlib.h>
 #include "logging.h"
+#include "../private.h"
 
 int main(int argc, char *argv[])
 {
@@ -17,7 +18,7 @@ int main(int argc, char *argv[])
        int flags[] = { NTDB_DEFAULT, NTDB_NOMMAP,
                        NTDB_CONVERT, NTDB_NOMMAP|NTDB_CONVERT };
 
-       plan_tests(sizeof(flags) / sizeof(flags[0]) * 28);
+       plan_tests(sizeof(flags) / sizeof(flags[0]) * 30);
        for (i = 0; i < sizeof(flags) / sizeof(flags[0]); i++) {
                ntdb = ntdb_open("run-open-multiple-times.ntdb",
                                 flags[i]|MAYBE_NOSYNC,
@@ -31,6 +32,7 @@ int main(int argc, char *argv[])
                                  O_RDWR|O_CREAT, 0600, &tap_log_attr);
                ok1(ntdb_check(ntdb, NULL, NULL) == 0);
                ok1(ntdb_check(ntdb2, NULL, NULL) == 0);
+               ok1((flags[i] & NTDB_NOMMAP) || ntdb2->file->map_ptr);
 
                /* Store in one, fetch in the other. */
                ok1(ntdb_store(ntdb, key, data, NTDB_REPLACE) == 0);
@@ -45,6 +47,7 @@ int main(int argc, char *argv[])
                /* OK, now close first one, check second still good. */
                ok1(ntdb_close(ntdb) == 0);
 
+               ok1((flags[i] & NTDB_NOMMAP) || ntdb2->file->map_ptr);
                ok1(ntdb_store(ntdb2, key, data, NTDB_REPLACE) == 0);
                ok1(ntdb_fetch(ntdb2, key, &d) == NTDB_SUCCESS);
                ok1(ntdb_deq(d, data));