On Fri, 26 Aug 2016 19:10:16 +0200, Adrian Buehlmann wrote: > On 2016-08-25 20:11, tt...@fb.com wrote: > > # HG changeset patch > > # User Tony Tung <tonyt...@merly.org> > > # Date 1472148645 25200 > > # Thu Aug 25 11:10:45 2016 -0700 > > # Node ID 625f3bb4833b47c4d35754fced91f37b077f7866 > > # Parent 2efc5a05d80a6d4253767f2ce0c2fb062ba83cb6 > > util: move checknlink away from the dependency of a hard-coded filename
> If I'm not mistaken, this will try to unlink file f1 while hardlinked > alias name f2 still has an open handle (opened using posixfile). > > This may not be a problem with posixfile, but Windows can have problems, > if a hardlinked file is deleted while it is still in open() state under > an alias name (see [1]). Ah, good catch. Tony, I'll drop this from the committed repo, so please make sure you have a copy of this revision before pulling from there. > I would thus recommend closing all files before unlinking. > > I'd additionally prefer doing cleanup things in reverse order, i.e. > close 'handle' on f2 first. > > So, I think I'd prefer doing the cleanup this way: > > # close files > for fi in (f2, f1): # intentionally in reverse order > if 'handle' in fi: > try: > fi['handle'].close() > except EnvironmentError: > pass > if 'fd' in fi: > try: > os.close(fi['fd']) > except EnvironmentError: > pass > > # unlink files > for fi in (f2, f1): # intentionally in reverse order > if 'path' in fi: > try: > os.unlink(fi['path']) > except EnvironmentError: > pass Perhaps we were trying to be too smart. Since raw fds are closed immediately, what we need in the finally block is just to close f2 object, then unlink f1 and f2. _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
