Skip to content

Conversation

gadfly3173
Copy link
Contributor

@gadfly3173 gadfly3173 commented May 22, 2025

To prevent potential race conditions where commit navigation fails due to stale branch information, this change unifies the branch refresh and commit refresh operations into a single thread.

Previously, separate refresh operations could lead to situations where:

  1. Commit refresh completes before branch refresh finishes
  2. Navigation then uses outdated branch information
  3. This results in incorrect commit jumps

The unified thread ensures branch information is always up-to-date when commits are refreshed, maintaining consistency between these related operations.

@goran-w
Copy link
Contributor

goran-w commented May 22, 2025

Good catch! Please check also all the occurrences of Task.Run(RefreshCommits) in class Repository...

Could the same issue occur for RefreshCommits vs RefreshTags? (If so, there's yet another case in class Watcher.)

@gadfly3173
Copy link
Contributor Author

Could the same issue occur for RefreshCommits vs RefreshTags? (If so, there's yet another case in class Watcher.)

Currently, only merge/pull/fetch operations need to navigate to the head commit of the branch after refreshing the commit. Therefore, it is necessary to update the branch information first only when refreshing the branch and triggering the refresh through the watcher.

@goran-w
Copy link
Contributor

goran-w commented May 22, 2025

Currently, only merge/pull/fetch operations need to navigate to the head commit of the branch after refreshing the commit.

I see - so there's no need to also patch Repository.RefreshAll() and Repository.MarkBranchesDirtyManually() where branches and commits are refreshed in parallel?

BTW, hopefully this will fix an issue I've noticed a few times, where a branch-badge is still shown at an older commit after an operation that should move it, and there even being two instances of the same branch-badge in the log-graph (one at the incorrect older commit and one at the correct newer one)!

@love-linger
Copy link
Collaborator

I do not like this change. I've pushed a new commit c112549 that query the changed branch head when we want to navigate it delayed (after commits have been refreshed)

@gadfly3173 gadfly3173 closed this May 23, 2025
@gadfly3173 gadfly3173 deleted the fix/watcher branch May 23, 2025 06:27
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