From 8ed9904df585e46878f891c746c30bfa5c54fed0 Mon Sep 17 00:00:00 2001 From: Michael Orlitzky Date: Thu, 3 Apr 2025 21:45:39 -0400 Subject: [PATCH] src/geoipyupdate/__init__.py: try os.replace() before shutil.move() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit It turns out that shutil.move() is not atomic, which can lead to issues when replacing the database under heavy load. There is an alternative in os.replace() that *is* atomic, but does not work across filesystems. For the best of both worlds, we now try os.replace(), and fall back to shutil.move() if it fails. Testing shows that os.replace() raises an OSError across multiple filesystems, so that's what we catch. The documentation doesn't specify one way or the other. Thanks to Paulo Magalhães for report. --- src/geoipyupdate/__init__.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/geoipyupdate/__init__.py b/src/geoipyupdate/__init__.py index ace4bdc..a9e3bf5 100644 --- a/src/geoipyupdate/__init__.py +++ b/src/geoipyupdate/__init__.py @@ -131,5 +131,12 @@ def main(): # minus whatever the user has set in his umask. os.chmod(g.name, 0o664 & ~old_umask) - # Overwrite the old database file with the new (gunzipped) one. - shutil.move(g.name, dbfile) + # Overwrite the old database file with the new (gunzipped) + # one. The os.replace() method is preferable because it is + # atomic, but it will raise an OSError if the source and + # destination are on different filesystems. We fall back + # to shutil.move in that case. + try: + os.replace(g.name, dbfile) + except OSError: + shutil.move(g.name, dbfile) -- 2.49.0