Skip to content

Fix stale Git diff updates after mode changes and close #9750

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.

Sign up for GitHub

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

Open
emilianbold wants to merge 2 commits into
base: master
Choose a base branch
from
+334 −19
Open

Fix stale Git diff updates after mode changes and close #9750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e332d4
Fix stale Git diff updates after mode changes and close
emilianbold Oct 25, 2026
e476dfb
Pass the diff preparation generation explicitly
emilianbold Oct 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 80 additions & 19 deletions ide/git/src/org/netbeans/modules/git/ui/diff/MultiDiffPanelController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.prefs.PreferenceChangeEvent;
Expand Down Expand Up @@ -167,6 +168,8 @@ public class MultiDiffPanelController implements ActionListener, PropertyChangeL
private final ApplyChangesTask changeTask = new ApplyChangesTask();
private final RequestProcessor.Task refreshNodesRPTask = RP.create(refreshNodesTask);
private final RequestProcessor.Task changeRPTask = RP.create(changeTask);
private final AtomicLong refreshGeneration = new AtomicLong();
private volatile boolean closed;
private static final String TEXT_DIFF = "text/x-diff"; //NOI18N
private GitProgressSupport multiTextDiffSupport;
private static final String PROP_SEARCH_CONTAINER = "diff.search.container"; //NOI18N
Expand Down Expand Up @@ -266,7 +269,7 @@ public MultiDiffPanelController (File file, Revision rev1, Revision rev2, int re
Map<File, Setup> localSetups = Collections.singletonMap(file, s);
setSetups(localSetups, getCookiesFromSetups(localSetups));
setDiffIndex(s, 0, false);
dpt = new DiffPrepareTask(setups.values().toArray(new Setup[setups.size()]));
dpt = new DiffPrepareTask(setups.values().toArray(new Setup[setups.size()]), refreshGeneration.get());
prepareTask = RP.create(dpt);
prepareTask.schedule(0);
}
Expand Down Expand Up @@ -426,12 +429,18 @@ UndoRedo getUndoRedo () {
}

public void componentClosed () {
if (closed) {
return;
}
closed = true;
refreshGeneration.incrementAndGet();
cancelBackgroundTasks();
setSetups(Collections.<File, Setup>emptyMap(), Collections.<File, EditorCookie>emptyMap());
prevAction.setEnabled(false);
nextAction.setEnabled(false);
cancelBackgroundTasks();
setups.clear();
editorCookies.clear();
currentSetup = null;

if (list != null) {
Git.getInstance().getFileStatusCache().removePropertyChangeListener(list);
Expand All @@ -448,6 +457,9 @@ private void cancelBackgroundTasks() {
if (multiTextDiffSupport != null) {
multiTextDiffSupport.cancel();
}
if (dpt != null) {
dpt.cancel();
}
if (prepareTask != null) {
prepareTask.cancel();
}
Expand All @@ -457,6 +469,11 @@ private void cancelBackgroundTasks() {
}
refreshNodesTask.cancel();
refreshNodesRPTask.cancel();
changeTask.cancel();
changeRPTask.cancel();
if (refreshComboTask != null) {
refreshComboTask.cancel();
}
}

void setFocused (boolean focused) {
Expand Down Expand Up @@ -641,17 +658,24 @@ private void setDisplayStatuses (EnumSet<Status> displayStatuses) {
}

private void setSetups (Map<File, Setup> setups, Map<File, EditorCookie> editorCookies) {
for (Map.Entry<File, Setup> e : this.setups.entrySet()) {
Setup setup = e.getValue();
closeSetups(this.setups);
this.setups.clear();
this.setups.putAll(setups);
this.editorCookies.clear();
this.editorCookies.putAll(editorCookies);
}

private void closeSetups(Map<File, Setup> setups) {
for (Setup setup : setups.values()) {
if (setup != null) {
DiffController view = setup.getView();
if (view != null) {
view.removePropertyChangeListener(this);
}
setup.getFirstSource().close();
setup.getSecondSource().close();
}
}
this.setups.clear();
this.setups.putAll(setups);
this.editorCookies.clear();
this.editorCookies.putAll(editorCookies);
}

private void onNextButton() {
Expand Down Expand Up @@ -1274,7 +1298,8 @@ void filesSelected (File... files) {

// <editor-fold defaultstate="collapsed" desc="refreshing tasks">
private void refreshNodes () {
if (context != null) {
if (context != null && !closed) {
refreshGeneration.incrementAndGet();
synchronized (changes) {
changes.clear();
}
Expand All @@ -1295,7 +1320,7 @@ private void refreshNodes () {

private void startPrepareTask (Setup activeSetup) {
if (setups.size() > 0) {
dpt = new DiffPrepareTask(setups.values().toArray(new Setup[setups.size()]));
dpt = new DiffPrepareTask(setups.values().toArray(new Setup[setups.size()]), refreshGeneration.get());
dpt.setSelectedSetup(activeSetup);
prepareTask = RP.create(dpt);
prepareTask.schedule(0);
Expand Down Expand Up @@ -1388,13 +1413,32 @@ private EditorCookie[] sort (Map<File, EditorCookie> cookies, DiffNode[] nodes)
}

private void setupsChanged (final Map<File, Setup> newSetups, final List<DiffNode> nodes) {
setupsChanged(newSetups, nodes, refreshGeneration.incrementAndGet());
}

private boolean isCurrent(long generation) {
return !closed && refreshGeneration.get() == generation;
}

private void setupsChanged (final Map<File, Setup> newSetups, final List<DiffNode> nodes, final long generation) {
if (!isCurrent(generation)) {
closeSetups(newSetups);
return;
}
final Map<File, EditorCookie> cookies = getCookiesFromSetups(newSetups);
final DiffNode[] nodeArray = nodes.toArray(new DiffNode[0]);
final Object modelDataList = fileListComponent.prepareModel(nodeArray);
final Object modelDataTree = fileTreeComponent.prepareModel(nodeArray);
Mutex.EVENT.readAccess(new Runnable() {
@Override
public void run() {
// Cancellation may happen after model preparation but before
// this EDT handoff. Never publish into a newer comparison or
// a window that has already closed.
if (!isCurrent(generation)) {
closeSetups(newSetups);
return;
}
dividerSet = false;
setSetups(newSetups, cookies);
fileListComponent.setModel(nodeArray, new HashMap<File, EditorCookie>(cookies), modelDataList);
Expand All @@ -1409,6 +1453,7 @@ private final class RefreshNodesTask implements Runnable, Cancellable {

@Override
public void run() {
final long generation = refreshGeneration.get();
canceled = false;
final List<DiffNode> nodes = new LinkedList<DiffNode>();
final Map<File, Setup> localSetups;
Expand All @@ -1421,10 +1466,11 @@ public void run() {
} else {
localSetups = getRevisionToRevisionSetups(nodes);
}
if (canceled) {
if (canceled || !isCurrent(generation)) {
closeSetups(localSetups);
return;
}
setupsChanged(localSetups, nodes);
setupsChanged(localSetups, nodes, generation);
}

@Override
Expand Down Expand Up @@ -1552,6 +1598,7 @@ private final class ApplyChangesTask implements Runnable, Cancellable {

@Override
public void run() {
final long generation = refreshGeneration.get();
canceled = false;
final Set<FileStatusCache.ChangedEvent> events;
synchronized (changes) {
Expand Down Expand Up @@ -1637,6 +1684,10 @@ public Map<File, DiffNode> run() {
Mutex.EVENT.readAccess(new Runnable() {
@Override
public void run() {
if (canceled || !isCurrent(generation)) {
closeSetups(localSetups);
return;
}
for (DiffNode node : toAdd) {
setups.put(node.getFile(), localSetups.get(node.getFile()));
EditorCookie cookie = cookies.get(node.getFile());
Expand Down Expand Up @@ -1705,6 +1756,9 @@ public void run () {
EventQueue.invokeLater(new Runnable() {
@Override
public void run () {
if (closed) {
return;
}
panel.cmbDiffTreeFirst.setModel(new DefaultComboBoxModel(modelRight.toArray(new Object[0])));
panel.cmbDiffTreeSecond.setModel(new DefaultComboBoxModel(modelLeft.toArray(new Object[0])));
panel.cmbDiffTreeFirst.setEnabled(true);
Expand All @@ -1719,28 +1773,32 @@ private class DiffPrepareTask implements Runnable, Cancellable {
private final Setup[] prepareSetups;
private Setup selectedSetup;
private volatile boolean canceled;
private final long generation;

public DiffPrepareTask(Setup[] prepareSetups) {
public DiffPrepareTask(Setup[] prepareSetups, long generation) {
assert EventQueue.isDispatchThread();
assert !Arrays.asList(prepareSetups).contains(null);
this.prepareSetups = prepareSetups;
this.selectedSetup = prepareSetups[0];
this.generation = generation;
}

@Override
public void run() {
canceled = false;
if (canceled || !isCurrent(generation)) {
return;
}
IOException exception = null;
DiffNode[] neighbourNodes = prepareSetupsToRefresh();
for (DiffNode n : neighbourNodes) {
final Setup s = n.getSetup();
if (canceled) return;
if (canceled || !isCurrent(generation)) return;
if (s.getView() != null) {
continue;
}
try {
s.initSources(); // slow network I/O
if (canceled) {
if (canceled || !isCurrent(generation)) {
return;
}
StreamSource ss1 = s.getFirstSource();
Expand All @@ -1749,14 +1807,17 @@ public void run() {
requestedLeftLine = getMatchingLine(ss2, ss1, requestedRightLine);
}
final DiffController view = DiffController.createEnhanced(ss1, ss2); // possibly executing slow external diff
view.addPropertyChangeListener(MultiDiffPanelController.this);
if (canceled) {
if (canceled || !isCurrent(generation)) {
return;
}
s.setView(view);
EventQueue.invokeLater(new Runnable() {
@Override
public void run() {
if (canceled || !isCurrent(generation)) {
return;
}
view.addPropertyChangeListener(MultiDiffPanelController.this);
s.setView(view);
if (s.equals(currentSetup)) {
setDiffIndex(s, 0, false);
}
Expand Down
Loading
Loading