fix(plugins): make the install marker durable before claim_install returns
Review nit from @karthik-indla on this PR. A hard kill between the O_EXCL open and the buffered write reaching disk left a marker that exists but parses to nothing: is_first_run reads it as claimed, so that install is never counted, and claim_version_change cannot read a version out of it. Not temp-and-rename, which is what the equivalent fixes in this stack use: the O_EXCL open is what makes this claim exclusive across concurrently starting sessions, and a rename would clobber rather than lose the race. The content is the part that needed making safe, so it is flushed and fsynced before the call returns. The recovery path stays as the backstop: _repair_install_state already rewrites an unparseable marker so version tracking resumes. 304 passed, 8 skipped. Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb
This commit is contained in:
@@ -370,6 +370,14 @@ def claim_install(was_empty: bool | None = None) -> str | None:
|
||||
},
|
||||
stream,
|
||||
)
|
||||
# Durable before this returns. The O_EXCL open is what makes the
|
||||
# claim exclusive, so it cannot be replaced by a temp-and-rename
|
||||
# without losing that, which leaves the content as the thing to make
|
||||
# safe. A kill between the open and this fsync used to leave a marker
|
||||
# that exists but parses to nothing: is_first_run reads it as claimed
|
||||
# and claim_version_change cannot read a version out of it.
|
||||
stream.flush()
|
||||
os.fsync(stream.fileno())
|
||||
except OSError:
|
||||
pass
|
||||
return "upgrade" if upgrading else "install"
|
||||
|
||||
@@ -370,6 +370,14 @@ def claim_install(was_empty: bool | None = None) -> str | None:
|
||||
},
|
||||
stream,
|
||||
)
|
||||
# Durable before this returns. The O_EXCL open is what makes the
|
||||
# claim exclusive, so it cannot be replaced by a temp-and-rename
|
||||
# without losing that, which leaves the content as the thing to make
|
||||
# safe. A kill between the open and this fsync used to leave a marker
|
||||
# that exists but parses to nothing: is_first_run reads it as claimed
|
||||
# and claim_version_change cannot read a version out of it.
|
||||
stream.flush()
|
||||
os.fsync(stream.fileno())
|
||||
except OSError:
|
||||
pass
|
||||
return "upgrade" if upgrading else "install"
|
||||
|
||||
@@ -370,6 +370,14 @@ def claim_install(was_empty: bool | None = None) -> str | None:
|
||||
},
|
||||
stream,
|
||||
)
|
||||
# Durable before this returns. The O_EXCL open is what makes the
|
||||
# claim exclusive, so it cannot be replaced by a temp-and-rename
|
||||
# without losing that, which leaves the content as the thing to make
|
||||
# safe. A kill between the open and this fsync used to leave a marker
|
||||
# that exists but parses to nothing: is_first_run reads it as claimed
|
||||
# and claim_version_change cannot read a version out of it.
|
||||
stream.flush()
|
||||
os.fsync(stream.fileno())
|
||||
except OSError:
|
||||
pass
|
||||
return "upgrade" if upgrading else "install"
|
||||
|
||||
@@ -370,6 +370,14 @@ def claim_install(was_empty: bool | None = None) -> str | None:
|
||||
},
|
||||
stream,
|
||||
)
|
||||
# Durable before this returns. The O_EXCL open is what makes the
|
||||
# claim exclusive, so it cannot be replaced by a temp-and-rename
|
||||
# without losing that, which leaves the content as the thing to make
|
||||
# safe. A kill between the open and this fsync used to leave a marker
|
||||
# that exists but parses to nothing: is_first_run reads it as claimed
|
||||
# and claim_version_change cannot read a version out of it.
|
||||
stream.flush()
|
||||
os.fsync(stream.fileno())
|
||||
except OSError:
|
||||
pass
|
||||
return "upgrade" if upgrading else "install"
|
||||
|
||||
@@ -370,6 +370,14 @@ def claim_install(was_empty: bool | None = None) -> str | None:
|
||||
},
|
||||
stream,
|
||||
)
|
||||
# Durable before this returns. The O_EXCL open is what makes the
|
||||
# claim exclusive, so it cannot be replaced by a temp-and-rename
|
||||
# without losing that, which leaves the content as the thing to make
|
||||
# safe. A kill between the open and this fsync used to leave a marker
|
||||
# that exists but parses to nothing: is_first_run reads it as claimed
|
||||
# and claim_version_change cannot read a version out of it.
|
||||
stream.flush()
|
||||
os.fsync(stream.fileno())
|
||||
except OSError:
|
||||
pass
|
||||
return "upgrade" if upgrading else "install"
|
||||
|
||||
@@ -370,6 +370,14 @@ def claim_install(was_empty: bool | None = None) -> str | None:
|
||||
},
|
||||
stream,
|
||||
)
|
||||
# Durable before this returns. The O_EXCL open is what makes the
|
||||
# claim exclusive, so it cannot be replaced by a temp-and-rename
|
||||
# without losing that, which leaves the content as the thing to make
|
||||
# safe. A kill between the open and this fsync used to leave a marker
|
||||
# that exists but parses to nothing: is_first_run reads it as claimed
|
||||
# and claim_version_change cannot read a version out of it.
|
||||
stream.flush()
|
||||
os.fsync(stream.fileno())
|
||||
except OSError:
|
||||
pass
|
||||
return "upgrade" if upgrading else "install"
|
||||
|
||||
@@ -370,6 +370,14 @@ def claim_install(was_empty: bool | None = None) -> str | None:
|
||||
},
|
||||
stream,
|
||||
)
|
||||
# Durable before this returns. The O_EXCL open is what makes the
|
||||
# claim exclusive, so it cannot be replaced by a temp-and-rename
|
||||
# without losing that, which leaves the content as the thing to make
|
||||
# safe. A kill between the open and this fsync used to leave a marker
|
||||
# that exists but parses to nothing: is_first_run reads it as claimed
|
||||
# and claim_version_change cannot read a version out of it.
|
||||
stream.flush()
|
||||
os.fsync(stream.fileno())
|
||||
except OSError:
|
||||
pass
|
||||
return "upgrade" if upgrading else "install"
|
||||
|
||||
Reference in New Issue
Block a user