Skip to content

Fix sparse-checkout crash due to custom virtual filesystem patches#639

Merged
derrickstolee merged 1 commit intomicrosoft:vfs-2.44.0from
derrickstolee:sparse-checkout-vfs-fix
Apr 15, 2024
Merged

Fix sparse-checkout crash due to custom virtual filesystem patches#639
derrickstolee merged 1 commit intomicrosoft:vfs-2.44.0from
derrickstolee:sparse-checkout-vfs-fix

Conversation

@derrickstolee
Copy link

This fixup! commit updates an old virtual filesystem patch to only check its custom logic if the virtual filesystem is enabled. When not enabled, the call to resolve_dtype() could cause a sparse-index expansion (ensure_full_index()) during an existing scan of the index, leading to use-after-free violations.

I verified that my local version with this fix avoids the crash in the case of the Office monorepo.

@derrickstolee derrickstolee self-assigned this Apr 11, 2024
Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

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

Looks obviously correct to me!

@derrickstolee
Copy link
Author

@dscho @jeffhostetler: The osx-(clang|gcc) test failures seem pretty consistent across three rebuilds, but also unrelated to my changes. Any idea what's going on there?

@dscho
Copy link
Member

dscho commented Apr 12, 2024

@dscho @jeffhostetler: The osx-(clang|gcc) test failures seem pretty consistent across three rebuilds, but also unrelated to my changes. Any idea what's going on there?

I think #640 will fix this.

@haeminlee0630
Copy link

@dscho @jeffhostetler it does look like at least one of the CI/osx-clang tests remain failing even after #640 was merged this morning. Any thoughts about this?

This avoids crashes when not using the virtualfilesystem feature.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho force-pushed the sparse-checkout-vfs-fix branch from 894122f to 6aabf8c Compare April 12, 2024 23:16
@dscho
Copy link
Member

dscho commented Apr 12, 2024

@dscho @jeffhostetler it does look like at least one of the CI/osx-clang tests remain failing even after #640 was merged this morning. Any thoughts about this?

I think it simply did not synchronize with the newest vfs-2.44.0 commit, probably because they did not clash with the changes in this here PR. I've rebased onto the newest vfs-2.44.0 commit, now CI should pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants