svn commit: r1702089 - /subversion/trunk/subversion/libsvn_subr/io.c

[ko...@apache.org]

Author: kotkov
Date: Wed Sep  9 19:14:04 2015
New Revision: 1702089

URL: http://svn.apache.org/r1702089
Log:
Explicitly release file locks on Windows, instead of relying on them to be
automatically unlocked by the OS.

Documentation for LockFileEx() [1] states that, although a process doesn't
necessarily have to release its file locks before closing the handle, it
is not recommended, and a better choice is to explicitly unlock them:
[[[
  If a process terminates with a portion of a file locked or closes a file
  that has outstanding locks, the locks are unlocked by the operating system.
  However, the time it takes for the operating system to unlock these locks
  depends upon available system resources. Therefore, it is recommended that
  your process explicitly unlock all files it has locked when it terminates.
  If this is not done, access to these files may be denied if the operating
  system has not yet unlocked them.
]]]

This changeset turns our common locking sequences from

  CreateFile → LockFile → (...) → CloseFile

    into

  CreateFile → LockFile → (...) → UnlockFileSingle → CloseFile

I conducted a couple of performance tests with svnadmin load, setrevprop,
simple commits, parallel short-living svnadmin instances doing work — i.e.,
tests where explicit unlocking could behave differently from delegating this
to OS.  There is no noticeable difference, at least on my machine.  So, we
do this not because it causes visible effects in common scenarios, but just
to follow the recommended practice and reduce the chance of encountering a
hard-to-diagnose problem.

Here is one of the performance tests that I used:
[[[
def lock_performance(sbox):
  "lock performance"

  sbox.build(create_wc=False)
  input_file = sbox.get_tempname()
  svntest.main.file_write(input_file, 'New log message')

  def setlog(n):
    svntest.actions.run_and_verify_svnadmin([], [], 'setrevprop', '-r1',
                                            sbox.repo_dir, 'prop' + str(n),
                                            input_file)
  import multiprocessing.dummy

  start = time.time()
  p = multiprocessing.dummy.Pool()
  results = p.map(setlog, range(2000))
  p.close()
  p.join();
  stop = time.time()
  logger.info('<TOTAL TIME = %.6f>' % (stop - start))
]]]

[1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa365203

Found by: ivan

* subversion/libsvn_subr/io.c
  (file_clear_locks): Always compile this function.
  (svn_io_lock_open_file, svn_io_unlock_open_file): Install and remove pool
   cleanup handlers on all platforms.  Update the corresponding comments.

Modified:
    subversion/trunk/subversion/libsvn_subr/io.c

Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1702089&r1=1702088&r2=1702089&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Wed Sep  9 19:14:04 2015
@@ -2272,7 +2272,6 @@ svn_io_is_file_executable(svn_boolean_t
 
 
 /*** File locking. ***/
-#if !defined(WIN32) && !defined(__OS2__)
 /* Clear all outstanding locks on ARG, an open apr_file_t *. */
 static apr_status_t
 file_clear_locks(void *arg)
@@ -2287,7 +2286,6 @@ file_clear_locks(void *arg)
 
   return 0;
 }
-#endif
 
 svn_error_t *
 svn_io_lock_open_file(apr_file_t *lockfile_handle,
@@ -2345,13 +2343,14 @@ svn_io_lock_open_file(apr_file_t *lockfi
         }
     }
 
-/* On Windows and OS/2 file locks are automatically released when
-   the file handle closes */
-#if !defined(WIN32) && !defined(__OS2__)
+  /* On Windows, a process may not release file locks before closing the
+     handle, and in this case the outstanding locks are unlocked by the OS.
+     However, this is not recommended, because the actual unlocking may be
+     postponed depending on available system resources.  We explicitly unlock
+     the file as a part of the pool cleanup handler. */
   apr_pool_cleanup_register(pool, lockfile_handle,
                             file_clear_locks,
                             apr_pool_cleanup_null);
-#endif
 
   return SVN_NO_ERROR;
 }
@@ -2375,11 +2374,7 @@ svn_io_unlock_open_file(apr_file_t *lock
     return svn_error_wrap_apr(apr_err, _("Can't unlock file '%s'"),
                               try_utf8_from_internal_style(fname, pool));
 
-/* On Windows and OS/2 file locks are automatically released when
-   the file handle closes */
-#if !defined(WIN32) && !defined(__OS2__)
   apr_pool_cleanup_kill(pool, lockfile_handle, file_clear_locks);
-#endif
 
   return SVN_NO_ERROR;
 }