File locks now work on Windows

release_docs/RELEASE.txt

@@ -384,6 +384,23 @@ New Features
 
     Library:
     --------
+    - File locking now works on Windows
+
+      Since version 1.10.0, the HDF5 library has used a file locking scheme
+      to help enforce one reader at a time accessing an HDF5 file, which can
+      be helpful when setting up readers and writers to use the single-
+      writer/multiple-readers (SWMR) access pattern.
+
+      In the past, this was only functional on POSIX systems where flock() or
+      fcntl() were present. Windows used a no-op stub that always succeeded.
+
+      HDF5 now uses LockFileEx() and UnlockFileEx() to lock the file using the
+      same scheme as POSIX systems. We lock the entire file when we set up the
+      locks (by passing DWORDMAX as both size parameters to LockFileEx()).
+
+        (DER - 2021/03/19, HDFFV-10191)
+
+
     - H5Epush_ret() now requires a trailing semicolon
 
         H5Epush_ret() is a function-like macro that has been changed to

src/H5Fint.c

@@ -1056,9 +1056,10 @@ H5F_prefix_open_file(H5F_t *primary_file, H5F_prefix_open_t prefix_type, const c
 htri_t
 H5F__is_hdf5(const char *name, hid_t fapl_id)
 {
-    H5FD_t *file      = NULL;        /* Low-level file struct                    */
-    haddr_t sig_addr  = HADDR_UNDEF; /* Addess of hdf5 file signature            */
-    htri_t  ret_value = FAIL;        /* Return value                             */
+    H5FD_t *      file      = NULL;        /* Low-level file struct            */
+    H5F_shared_t *shared    = NULL;        /* Shared part of file              */
+    haddr_t       sig_addr  = HADDR_UNDEF; /* Addess of hdf5 file signature    */
+    htri_t        ret_value = FAIL;        /* Return value                     */
 
     FUNC_ENTER_PACKAGE
 
@@ -1069,10 +1070,20 @@ H5F__is_hdf5(const char *name, hid_t fapl_id)
     if (NULL == (file = H5FD_open(name, H5F_ACC_RDONLY, fapl_id, HADDR_UNDEF)))
         HGOTO_ERROR(H5E_FILE, H5E_CANTINIT, FAIL, "unable to open file")
 
-    /* The file is an hdf5 file if the hdf5 file signature can be found */
-    if (H5FD_locate_signature(file, &sig_addr) < 0)
-        HGOTO_ERROR(H5E_FILE, H5E_NOTHDF5, FAIL, "error while trying to locate file signature")
-    ret_value = (HADDR_UNDEF != sig_addr);
+    /* If the file is already open, it's an HDF5 file
+     *
+     * If the file is open with an exclusive lock on an operating system that enforces
+     * mandatory file locks (like Windows), creating a new file handle and attempting
+     * to read through it will fail so we have to try this first.
+     */
+    if ((shared = H5F__sfile_search(file)) != NULL)
+        ret_value = TRUE;
+    else {
+        /* The file is an HDF5 file if the HDF5 file signature can be found */
+        if (H5FD_locate_signature(file, &sig_addr) < 0)
+            HGOTO_ERROR(H5E_FILE, H5E_NOTHDF5, FAIL, "error while trying to locate file signature")
+        ret_value = (HADDR_UNDEF != sig_addr);
+    }
 
 done:
     /* Close the file */
@@ -1782,7 +1793,7 @@ H5F_open(const char *name, unsigned flags, hid_t fcpl_id, hid_t fapl_id)
     FUNC_ENTER_NOAPI(NULL)
 
     /*
-     * If the driver has a `cmp' method then the driver is capable of
+     * If the driver has a 'cmp' method then the driver is capable of
      * determining when two file handles refer to the same file and the
      * library can insure that when the application opens a file twice
      * that the two handles coordinate their operations appropriately.
@@ -3718,10 +3729,17 @@ H5F__start_swmr_write(H5F_t *f)
     setup = TRUE;
 
     /* Place an advisory lock on the file */
-    if (H5F_USE_FILE_LOCKING(f))
+    if (H5F_USE_FILE_LOCKING(f)) {
+        /* Have to unlock on Windows as Win32 doesn't support changing the lock
+         * type (exclusive vs shared) with a second call.
+         */
+        if (H5FD_unlock(f->shared->lf) < 0) {
+            HGOTO_ERROR(H5E_FILE, H5E_CANTUNLOCKFILE, FAIL, "unable to unlock the file")
+        }
         if (H5FD_lock(f->shared->lf, TRUE) < 0) {
             HGOTO_ERROR(H5E_FILE, H5E_CANTLOCKFILE, FAIL, "unable to lock the file")
         }
+    }
 
     /* Mark superblock as dirty */
     if (H5F_super_dirty(f) < 0)

src/H5Tconv.c

@@ -3843,7 +3843,7 @@ H5T__conv_i_i(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, size_t nelmts, siz
     size_t         half_size;            /*half the type size        */
     size_t         olap;                 /*num overlapping elements    */
     uint8_t *      s, *sp, *d, *dp;      /*source and dest traversal ptrs*/
-    uint8_t *      src_rev = NULL;       /*order-reversed source buffer  */
+    uint8_t *      src_rev  = NULL;      /*order-reversed source buffer  */
     uint8_t        dbuf[64] = {0};       /*temp destination buffer    */
     size_t         first;
     ssize_t        sfirst;                   /*a signed version of `first'    */
@@ -4286,7 +4286,7 @@ H5T__conv_f_f(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, size_t nelmts, siz
     size_t       olap;                 /*num overlapping elements    */
     ssize_t      bitno = 0;            /*bit number            */
     uint8_t *    s, *sp, *d, *dp;      /*source and dest traversal ptrs*/
-    uint8_t *    src_rev = NULL;       /*order-reversed source buffer  */
+    uint8_t *    src_rev  = NULL;      /*order-reversed source buffer  */
     uint8_t      dbuf[64] = {0};       /*temp destination buffer    */
     uint8_t      tmp1, tmp2;           /*temp variables for swapping bytes*/
 
@@ -8401,7 +8401,7 @@ H5T__conv_f_i(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, size_t nelmts, siz
     size_t       tsize;           /*type size for swapping bytes  */
     size_t       olap;            /*num overlapping elements    */
     uint8_t *    s, *sp, *d, *dp; /*source and dest traversal ptrs*/
-    uint8_t *    src_rev = NULL;  /*order-reversed source buffer  */
+    uint8_t *    src_rev  = NULL; /*order-reversed source buffer  */
     uint8_t      dbuf[64] = {0};  /*temp destination buffer    */
     uint8_t      tmp1, tmp2;      /*temp variables for swapping bytes*/
 
@@ -9027,7 +9027,7 @@ H5T__conv_i_f(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, size_t nelmts, siz
     size_t       tsize;           /*type size for swapping bytes  */
     size_t       olap;            /*num overlapping elements    */
     uint8_t *    s, *sp, *d, *dp; /*source and dest traversal ptrs*/
-    uint8_t *    src_rev = NULL;  /*order-reversed source buffer  */
+    uint8_t *    src_rev  = NULL; /*order-reversed source buffer  */
     uint8_t      dbuf[64] = {0};  /*temp destination buffer    */
     uint8_t      tmp1, tmp2;      /*temp variables for swapping bytes*/
 

src/H5system.c

@@ -618,40 +618,43 @@ c99_vsnprintf(char *str, size_t size, const char *format, va_list ap)
  *-------------------------------------------------------------------------
  */
 int
-Wflock(int H5_ATTR_UNUSED fd, int H5_ATTR_UNUSED operation)
+Wflock(int fd, int operation)
 {
 
-/* This is a no-op while we implement a Win32 VFD */
-#if 0
-int
-Wflock(int fd, int operation) {
-
-    HANDLE          hFile;
-    DWORD           dwFlags = LOCKFILE_FAIL_IMMEDIATELY;
-    DWORD           dwReserved = 0;
-                    /* MAXDWORD for entire file */
-    DWORD           nNumberOfBytesToLockLow = MAXDWORD;
-    DWORD           nNumberOfBytesToLockHigh = MAXDWORD;
-                    /* Must initialize OVERLAPPED struct */
-    OVERLAPPED      overlapped = {0};
+    HANDLE hFile;
+    DWORD  dwFlags    = LOCKFILE_FAIL_IMMEDIATELY;
+    DWORD  dwReserved = 0;
+    /* MAXDWORD locks the entire file */
+    DWORD nNumberOfBytesToLockLow  = MAXDWORD;
+    DWORD nNumberOfBytesToLockHigh = MAXDWORD;
+    /* Must initialize OVERLAPPED struct */
+    OVERLAPPED overlapped = {0};
 
     /* Get Windows HANDLE */
-    hFile = _get_osfhandle(fd);
+    if (INVALID_HANDLE_VALUE == (hFile = (HANDLE)_get_osfhandle(fd)))
+        return -1;
 
     /* Convert to Windows flags */
-    if(operation & LOCK_EX)
+    if (operation & LOCK_EX)
         dwFlags |= LOCKFILE_EXCLUSIVE_LOCK;
 
     /* Lock or unlock */
-    if(operation & LOCK_UN)
-        if(0 == UnlockFileEx(hFile, dwReserved, nNumberOfBytesToLockLow,
-                            nNumberOfBytesToLockHigh, &overlapped))
-            return -1;
-    else
-        if(0 == LockFileEx(hFile, dwFlags, dwReserved, nNumberOfBytesToLockLow,
-                            nNumberOfBytesToLockHigh, &overlapped))
+    if (operation & LOCK_UN) {
+        if (0 ==
+            UnlockFileEx(hFile, dwReserved, nNumberOfBytesToLockLow, nNumberOfBytesToLockHigh, &overlapped)) {
+            /* Attempting to unlock an already unlocked file will fail and this can happen
+             * in H5Fstart_swmr_write(). For now, just ignore the "error" (error code: 0x9e / 158).
+             */
+            if (GetLastError() != 158)
+                return -1;
+        }
+    }
+    else {
+        if (0 == LockFileEx(hFile, dwFlags, dwReserved, nNumberOfBytesToLockLow, nNumberOfBytesToLockHigh,
+                            &overlapped))
             return -1;
-#endif /* 0 */
+    }
+
     return 0;
 } /* end Wflock() */
 

test/tfile.c

@@ -1666,6 +1666,29 @@ test_file_is_accessible(const char *env_h5_drvr)
     is_hdf5 = H5Fis_accessible(filename, fapl_id);
     VERIFY(is_hdf5, TRUE, "H5Fis_accessible");
 
+    /*****************************************/
+    /* Newly created file that is still open */
+    /*****************************************/
+
+    /* On Windows, file locking is mandatory so this check ensures that
+     * H5Fis_accessible() works on files that have an exclusive lock.
+     * Previous versions of this API call created an additional file handle
+     * and attempted to read through it, which will not work when locks
+     * are enforced by the OS.
+     */
+
+    /* Create a file and hold it open */
+    fid = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, fapl_id);
+    CHECK(fid, H5I_INVALID_HID, "H5Fcreate");
+
+    /* Verify that the file is an HDF5 file */
+    is_hdf5 = H5Fis_accessible(filename, fapl_id);
+    VERIFY(is_hdf5, TRUE, "H5Fis_accessible");
+
+    /* Close file */
+    ret = H5Fclose(fid);
+    CHECK(ret, FAIL, "H5Fclose");
+
     /*******************************/
     /* Non-default user block size */
     /*******************************/

tools/src/h5repack/h5repack_copy.c

@@ -304,14 +304,6 @@ copy_objects(const char *fnamein, const char *fnameout, pack_opt_t *options)
     if ((fidout = H5Fcreate(fnameout, H5F_ACC_TRUNC, fcpl, options->fout_fapl)) < 0)
         H5TOOLS_GOTO_ERROR((-1), "H5Fcreate could not create file <%s>:", fnameout);
 
-    /*-------------------------------------------------------------------------
-     * write a new user block if requested
-     *-------------------------------------------------------------------------
-     */
-    if (options->ublock_size > 0)
-        if (copy_user_block(options->ublock_filename, fnameout, options->ublock_size) < 0)
-            H5TOOLS_GOTO_ERROR((-1), "Could not copy user block. Exiting...");
-
     /*-------------------------------------------------------------------------
      * get list of objects
      *-------------------------------------------------------------------------
@@ -346,27 +338,60 @@ copy_objects(const char *fnamein, const char *fnameout, pack_opt_t *options)
     }
 
     /*-------------------------------------------------------------------------
-     * write only the input file user block if there is no user block file input
+     * Close the file and everything in it so the lock is removed
+     *-------------------------------------------------------------------------
+     */
+    if (H5Pclose(fcpl) < 0)
+        H5TOOLS_GOTO_ERROR((-1), "could not close fcpl");
+    if (H5Pclose(options->fout_fapl) < 0)
+        H5TOOLS_GOTO_ERROR((-1), "could not close fcpl");
+    options->fout_fapl = H5P_DEFAULT;
+    if (H5Pclose(gcpl_in) < 0)
+        H5TOOLS_GOTO_ERROR((-1), "could not close fcpl");
+    if (H5Gclose(grp_in) < 0)
+        H5TOOLS_GOTO_ERROR((-1), "could not close fcpl");
+    if (H5Fclose(fidout) < 0)
+        H5TOOLS_GOTO_ERROR((-1), "could not close fcpl");
+    if (H5Fclose(fidin) < 0)
+        H5TOOLS_GOTO_ERROR((-1), "could not close fcpl");
+
+    /*-------------------------------------------------------------------------
+     * NOTE: The userblock MUST be written out AFTER the file is closed or
+     * the file locking will cause failures on Windows, where file locks
+     * are mandatory, not advisory.
+     *-------------------------------------------------------------------------
+     */
+
+    /*-------------------------------------------------------------------------
+     * Write a new user block if requested, using the input file user block if
+     * there is no separate user block file input
      *-------------------------------------------------------------------------
      */
 
-    if (ub_size > 0 && options->ublock_size == 0)
+    if (options->ublock_size > 0) {
+        if (copy_user_block(options->ublock_filename, fnameout, options->ublock_size) < 0)
+            H5TOOLS_GOTO_ERROR((-1), "Could not copy user block. Exiting...");
+    }
+    else if (ub_size > 0 && options->ublock_size == 0) {
         if (copy_user_block(fnamein, fnameout, ub_size) < 0)
             H5TOOLS_GOTO_ERROR((-1), "Could not copy user block. Exiting...");
+    }
 
 done:
-    H5E_BEGIN_TRY
-    {
-        H5Pclose(fcpl);
-        H5Pclose(options->fout_fapl);
-        options->fout_fapl = H5P_DEFAULT;
-        H5Pclose(gcpl_in);
-        H5Gclose(grp_in);
-        H5Pclose(fcpl_in);
-        H5Fclose(fidout);
-        H5Fclose(fidin);
+    if (-1 == ret_value) {
+        H5E_BEGIN_TRY
+        {
+            H5Pclose(fcpl);
+            H5Pclose(options->fout_fapl);
+            options->fout_fapl = H5P_DEFAULT;
+            H5Pclose(gcpl_in);
+            H5Gclose(grp_in);
+            H5Pclose(fcpl_in);
+            H5Fclose(fidout);
+            H5Fclose(fidin);
+        }
+        H5E_END_TRY;
     }
-    H5E_END_TRY;
     if (travt)
         trav_table_free(travt);