Patchwork [remotenames] locks: take wlock for file writes

login
register
mail settings
Submitter Durham Goode
Date July 14, 2017, 10:10 p.m.
Message ID <1c7d5c611f3fb5ba790e.1500070212@dev111.prn1.facebook.com>
Download mbox | patch
Permalink /patch/22383/
State Accepted
Headers show

Comments

Durham Goode - July 14, 2017, 10:10 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1500070111 25200
#      Fri Jul 14 15:08:31 2017 -0700
# Node ID 1c7d5c611f3fb5ba790eca5ca9d41c3398ba730d
# Parent  75df1d072f09392fb0b7314c856fb9689daa7c91
locks: take wlock for file writes

There are new warnings in upstream that catch vfs writes when the wlock isn't
taken. Remotenames triggers a couple of these. Let's take the lock. In both
cases, it happens during normal write operations (bookmark setting, and pull) so
we don't have to worry about this change causing locks during read operations.
Sean Farley - July 15, 2017, 8:22 p.m.
Durham Goode <durham@fb.com> writes:

> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1500070111 25200
> #      Fri Jul 14 15:08:31 2017 -0700
> # Node ID 1c7d5c611f3fb5ba790eca5ca9d41c3398ba730d
> # Parent  75df1d072f09392fb0b7314c856fb9689daa7c91
> locks: take wlock for file writes
>
> There are new warnings in upstream that catch vfs writes when the wlock isn't
> taken. Remotenames triggers a couple of these. Let's take the lock. In both
> cases, it happens during normal write operations (bookmark setting, and pull) so
> we don't have to worry about this change causing locks during read operations.

Queued, thanks!

Patch

diff --git a/remotenames.py b/remotenames.py
--- a/remotenames.py
+++ b/remotenames.py
@@ -167,10 +167,11 @@  def expull(orig, repo, remote, *args, **
     pullremotenames(repo, remote, bookmarks)
     if repo.vfs.exists(_selectivepullenabledfile):
         if not _isselectivepull(repo.ui):
-            repo.vfs.unlink(_selectivepullenabledfile)
+            with repo.wlock():
+                repo.vfs.unlink(_selectivepullenabledfile)
     else:
         if _isselectivepull(repo.ui):
-            with repo.vfs(_selectivepullenabledfile, 'w') as f:
+            with repo.wlock(), repo.vfs(_selectivepullenabledfile, 'w') as f:
                 f.write('enabled') # content doesn't matter
     return res
 
@@ -1164,11 +1165,12 @@  def _readtracking(repo):
     return tracking
 
 def _writetracking(repo, tracking):
-    data = ''
-    for book, track in tracking.iteritems():
-        data += '%s %s\n' % (book, track)
-    vfs = shareawarevfs(repo)
-    vfs.write('bookmarks.tracking', data)
+    with repo.wlock():
+        data = ''
+        for book, track in tracking.iteritems():
+            data += '%s %s\n' % (book, track)
+        vfs = shareawarevfs(repo)
+        vfs.write('bookmarks.tracking', data)
 
 def _removetracking(repo, bookmarks):
     tracking = _readtracking(repo)