FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Potential Race Condition Fix - OS Rename & Chmod - PermissionError by DEKHTIARJonathan · Pull Request #115 · gitpython-developers/gitdb · GitHub

This repository was archived by the owner on Jul 26, 2026. It is now read-only.
/ gitdb Public archive

Potential Race Condition Fix - OS Rename & Chmod - PermissionError #115

Merged
Byron merged 2 commits into from
Jan 2, 2025
+18 −4
Conversation 21 Commits 2 Checks 0 Files changed 1
Merged

Potential Race Condition Fix - OS Rename & Chmod - PermissionError #115

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
74a0eab
Potential Race Condition Fix - OS Rename & Chmod
DEKHTIARJonathan Dec 24, 2024
b71e273
Update gitdb/db/loose.py
DEKHTIARJonathan Dec 30, 2024
File filter

Filter by extension

Filter by extension .py  (1) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
22 changes: 18 additions & 4 deletions gitdb/db/loose.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import tempfile
import os
import sys
import time


__all__ = ('LooseObjectDB', )
Expand Down Expand Up @@ -205,7 +206,7 @@ def store(self, istream):
# END assure target stream is closed
except:
if tmp_path:
Comment thread
DEKHTIARJonathan marked this conversation as resolved.
os.remove(tmp_path)
remove(tmp_path)
raise
# END assure tmpfile removal on error

Expand All @@ -228,9 +229,22 @@ def store(self, istream):
rename(tmp_path, obj_path)
# end rename only if needed

# make sure its readable for all ! It started out as rw-- tmp file
# but needs to be rwrr
chmod(obj_path, self.new_objects_mode)
# Ensure rename is actually done and file is stable
# Retry up to 14 times - exponential wait & retry in ms.
# The total maximum wait time is 1000ms, which should be vastly enough for the
# OS to return and commit the file to disk.
for exp_backoff_ms in [1, 4, 9, 16, 25, 36, 49, 64, 81, 100, 121, 144, 169, 181]:
Comment on lines +232 to +236

EliahKagan Jan 4, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

These timings seem reasonable, but I believe this is quadratic backoff rather than exponential backoff. (In exponential backoff, delays are a fixed base, often 2, raised to the power of a number that increases by one with each attempt. Here, the number that increases by one with each attempt is the base, raised to a fixed power of 2.) I mention this only in case true exponential backoff is intended or required.

Byron Jan 5, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

That is very true! Since I copied these values from gitoxide where I implemented that based on what Git does. So most definitely it was me misnaming this, as Git correctly calls it quadratic.

A PR would definitely be welcome that fixes the name in gitoxide.

EliahKagan Jan 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I've opened GitoxideLabs/gitoxide#1815 to fix that in gitoxide. While I was at it, I also opened #119 to make the much more minor correction here.
with suppress(PermissionError):
# make sure its readable for all ! It started out as rw-- tmp file
# but needs to be rwrr
chmod(obj_path, self.new_objects_mode)
break
time.sleep(exp_backoff_ms / 1000.0)
else:
raise PermissionError(
"Impossible to apply `chmod` to file {}".format(obj_path)
)

# END handle dry_run

istream.binsha = hex_to_bin(hexsha)
Expand Down
Back | FazBrowse Home | New Git URL