Fix racy fsmonitor #2203
Merged
Fix racy fsmonitor #2203
Conversation
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
We will need to pass down the `struct index_state` to `mark_fsmonitor_valid()` for an upcoming bug fix, and this here function calls that there function, so we need to extend the signature of `fill_stat_cache_info()` first. Signed-off-by: Johannes Schindelin <[email protected]>
Without this bug fix, t7519's four "status doesn't detect unreported modifications" test cases would fail occasionally (and, oddly enough, *a lot* more frequently on Windows). The reason is that these test cases intentionally use the side effect of `git status` to re-write the index if any updates were detected: they first clean the worktree, run `git status` to update the index as well as show the output to the casual reader, then make the worktree dirty again and expect no changes to reported if running with a mocked fsmonitor hook. The problem with this strategy was that the index was written during said `git status` on the clean worktree for the *wrong* reason: not because the index was marked as changed (it wasn't), but because the recorded mtimes were racy with the index' own mtime. As the mtime granularity on Windows is 100 nanoseconds (see e.g. https://docs.microsoft.com/en-us/windows/desktop/SysInfo/file-times), the mtimes of the files are often enough *not* racy with the index', so that that `git status` call currently does not always update the index (including the fsmonitor extension), causing the test case to fail. The obvious fix: if we change *any* index entry's `CE_FSMONITOR_VALID` flag, we should also mark the index as changed. That will cause the index to be written upon `git status`, *including* an updated fsmonitor extension. Side note: Even though the reader might think that the t7519 issue should be *much* more prevalent on Linux, given that the ext4 filesystem (that seems to be used by every Linux distribution) stores mtimes in nanosecond precision. However, ext4 uses `current_kernel_time()` (see https://unix.stackexchange.com/questions/11599#comment762968_11599; it is *amazingly* hard to find any proper source of information about such ext4 questions) whose accuracy seems to depend on many factors but is safely worse than the 100-nanosecond granularity of NTFS (again, it is *horribly* hard to find anything remotely authoritative about this question). So it seems that the racy index condition that hid the bug fixed by this patch simply is a lot more likely on Linux than on Windows. But not impossible ;-) Signed-off-by: Johannes Schindelin <[email protected]>
|
Corresponding GitGitGadget PR: gitgitgadget#223 |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
dscho
added a commit
to dscho/git
that referenced
this pull request
May 25, 2019
…-gfw Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
that referenced
this pull request
May 25, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
that referenced
this pull request
May 25, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
that referenced
this pull request
May 25, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
that referenced
this pull request
May 25, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
that referenced
this pull request
May 26, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
that referenced
this pull request
May 29, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
to dscho/git
that referenced
this pull request
May 31, 2019
…-gfw Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jun 3, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
that referenced
this pull request
Jun 3, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
that referenced
this pull request
Jun 3, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
that referenced
this pull request
Jun 4, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
that referenced
this pull request
Jun 8, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jun 8, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
that referenced
this pull request
Jun 8, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
that referenced
this pull request
Jun 21, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jul 8, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jul 11, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci
pushed a commit
that referenced
this pull request
Jul 23, 2019
Fix racy fsmonitor The `t7519-status-fsmonitor.sh` tests became a *lot* more flaky with the recent fsmonitor fix (`js/fsmonitor-refresh-after-discarding-index`). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit *only* on Windows. The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the `has_racy_timestamp()` test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux). This fixes gitgitgadget#197 Signed-off-by: Johannes Schindelin <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
t7519-status-fsmonitor.shtests became a lot more flaky with the recent fsmonitor fix (js/fsmonitor-refresh-after-discarding-index). That fix, however, did not introduce the flakiness, but it just made it much more likely to be hit. And it seemed to be hit only on Windows.The reason, though, is that the fsmonitor feature failed to mark the in-memory index as changed, i.e. in need of writing, and it was the
has_racy_timestamp()test that hid this bug in most cases (although a lot less on Windows, where the files' mtimes are actually a lot more accurate than on Linux).This fixes gitgitgadget#197