Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib/model: Correct virtual mtime handling (fixes #3516) #3519

Closed
wants to merge 1 commit into from
Closed

lib/model: Correct virtual mtime handling (fixes #3516) #3519

wants to merge 1 commit into from

Conversation

calmh
Copy link
Member

@calmh calmh commented Aug 16, 2016

Purpose

We previously set the mtime on the temp file, and then renamed it to the real path. Unfortunately that means we'd save the real timestamp under the under the temp name ".syncthing.foo.tmp" when the actual file that we will look up on the next scan is "foo". This moves the Chtimes later, ensuring that it gets recorded correctly under the right name.

Testing

Verified in my FreeBSD + Mac setup. Unfortunately we don't have a good way to test the puller, especially not cross platform... :/

We previously set the mtime on the temp file, and then renamed it to the
real path. Unfortunately that means we'd save the real timestamp under
the under the temp name ".syncthing.foo.tmp" when the actual file that
we will look up on the next scan is "foo". This moves the Chtimes later,
ensuring that it gets recorded correctly under the right name.
@AudriusButkevicius
Copy link
Member

This introduces a race.

T1: Rename to realName
T2: Scan realName detect mtime change
T1: Adjust mtime

@calmh
Copy link
Member Author

calmh commented Aug 16, 2016

We don't scan in parallell with pulling, or do you mean that someone not-syncthing might make that change in the meantime?

@AudriusButkevicius
Copy link
Member

@st-review merge

@st-review
Copy link

👌 Merged as 0b14751. Thanks, @calmh!

st-review pushed a commit that referenced this pull request Aug 16, 2016
We previously set the mtime on the temp file, and then renamed it to the
real path. Unfortunately that means we'd save the real timestamp under
the under the temp name ".syncthing.foo.tmp" when the actual file that
we will look up on the next scan is "foo". This moves the Chtimes later,
ensuring that it gets recorded correctly under the right name.

GitHub-Pull-Request: #3519
@st-review st-review closed this Aug 16, 2016
@calmh calmh deleted the fix-3516 branch October 5, 2016 08:28
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Aug 17, 2017
@syncthing syncthing locked and limited conversation to collaborators Aug 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants