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

Fix issue #267 - Reclone broken on windows #339

Closed
wants to merge 2 commits into from

Conversation

Xeckt
Copy link
Contributor

@Xeckt Xeckt commented Jan 16, 2025

Hopefully closes #267

While I wanted to stick to fixing the issue at hand, it became apparent that the issue was with the mutex guards. While this issue could also occur on Linux, it was more unlikely due to the logical difference in their kernels on object handling. So the way we handled the Repository mutex was problematic.

What I hoped to achieve for full atomicity and cross-platform support is making sure all file handles are dropped regardless, and keeping hold of the mutex guard throughout the whole reclone process. It was quite difficult to think about how to do in this situation. I've had @TheKrol test the code and it worked for him so here's the breakdown:

  • Acquire the mutex lock (or guard, whichever you prefer to name it)
  • Make sure we catch if the mutex guard is poisoned or locked first, if so bail
  • Clean and remake the temp repo path explicitly
  • Clone into temp repo path, ready for replacement
  • Create a dummy repository that won't be used nor have file handles at an OS level since nothing is accessing it, so we can use it to replace the mutex and force the OS to drop handles, making sure it works on all OS platforms
  • drop(replace(&mut *lock, dummy)); - takes the old repo out of the mutex, puts dummy in it's place, returns it, then drops it from the mutex, will keep the mutex guard itself in tact so we keep hold of the thread while releasing all handles to the old repo we want to replace
  • Rename, and move the newly recloned repo into the mutex lock

I wanted to make sure we kept hold of the mutex guard the whole time, as to achieve atomicity.

I'm still unsure if this is a good approach but it made sense to me, since the Repository struct isn't using tokio so we can't necessarily hold other threads while we work on it.

Copy link
Contributor

@zleyyij zleyyij left a comment

Choose a reason for hiding this comment

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

Not tested yet, just one point of discussion

backend/src/git.rs Show resolved Hide resolved
@Xeckt Xeckt closed this Jan 17, 2025
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.

Reclone is broken on windows
2 participants