Skip to content

Guard access to elements/batchedEntries for MT access #1246

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

Merged
merged 1 commit into from
Oct 25, 2023
Merged

Guard access to elements/batchedEntries for MT access #1246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion bundles/org.eclipse.ui.views.log/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %name
Bundle-SymbolicName: org.eclipse.ui.views.log;singleton:=true
Bundle-Version: 1.4.100.qualifier
Bundle-Version: 1.4.200.qualifier
Bundle-Activator: org.eclipse.ui.internal.views.log.Activator
Bundle-Vendor: %provider-name
Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.29.0,4.0.0)",
Expand Down
76 changes: 48 additions & 28 deletions bundles/org.eclipse.ui.views.log/src/org/eclipse/ui/internal/views/log/LogView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,12 @@ public class LogView extends ViewPart implements LogListener {

private ServiceTracker<LogReaderService, LogReaderService> logReaderServiceTracker;

private List<AbstractEntry> elements;
private final List<AbstractEntry> elements;
private Map<Object, Group> groups;
private LogSession currentSession;

private List<LogEntry> batchedEntries;
private boolean batchEntries;
private final List<LogEntry> batchedEntries;
private volatile boolean batchEntries;

private Clipboard fClipboard;

Expand Down Expand Up @@ -244,10 +244,11 @@ public void perspectiveChanged(IWorkbenchPage page, IPerspectiveDescriptor persp

if (part.equals(LogView.this)) {
if (changeId.equals(IWorkbenchPage.CHANGE_VIEW_SHOW)) {
if (!batchedEntries.isEmpty()) {
pushBatchedEntries();
synchronized (batchedEntries) {
if (!batchedEntries.isEmpty()) {
pushBatchedEntries();
}
}

batchEntries = false;
} else if (changeId.equals(IWorkbenchPage.CHANGE_VIEW_HIDE)) {
batchEntries = true;
Expand Down Expand Up @@ -834,10 +835,12 @@ public AbstractEntry[] getElements() {

public void handleClear() {
BusyIndicator.showWhile(fTree.getDisplay(), () -> {
elements.clear();
groups.clear();
if (currentSession != null) {
currentSession.removeAllChildren();
synchronized (elements) {
elements.clear();
groups.clear();
if (currentSession != null) {
currentSession.removeAllChildren();
}
}
asyncRefresh(false);
resetDialogButtons();
Expand Down Expand Up @@ -888,10 +891,12 @@ private CompletableFuture<List<LogEntry>> fetchLogEntries() {
}

private void updateLogViewer(List<LogEntry> entries) {
elements.clear();
groups.clear();
group(entries);
limitEntriesCount();
synchronized (elements) {
elements.clear();
groups.clear();
group(entries);
limitEntriesCount();
}
setContentDescription(getTitleSummary());

asyncRefresh(false);
Expand Down Expand Up @@ -1082,7 +1087,9 @@ public void logged(org.osgi.service.log.LogEntry input) {
if (batchEntries) {
// create LogEntry immediately to don't loose IStatus creation date.
LogEntry entry = betterInput != null ? createLogEntry(betterInput) : createLogEntry(input);
batchedEntries.add(entry);
synchronized (batchedEntries) {
batchedEntries.add(entry);
}
return;
}

Expand All @@ -1093,11 +1100,16 @@ public void logged(org.osgi.service.log.LogEntry input) {
} else {
LogEntry entry = betterInput != null ? createLogEntry(betterInput) : createLogEntry(input);

if (!batchedEntries.isEmpty()) {
boolean useBatchedEntries;
synchronized (batchedEntries) {
useBatchedEntries = !batchedEntries.isEmpty();
// batch new entry as well, to have only one asyncRefresh()
batchedEntries.add(entry);
pushBatchedEntries();
} else {
if (useBatchedEntries) {
batchedEntries.add(entry);
pushBatchedEntries();
}
}
if (!useBatchedEntries) {
pushEntry(entry);
asyncRefresh(true);
}
Expand All @@ -1111,11 +1123,14 @@ private void pushBatchedEntries() {
Job job = new Job(Messages.LogView_AddingBatchedEvents) {
@Override
protected IStatus run(IProgressMonitor monitor) {
for (int i = 0; i < batchedEntries.size(); i++) {
if (!monitor.isCanceled()) {
LogEntry entry = batchedEntries.get(i);
pushEntry(entry);
batchedEntries.remove(i);
synchronized (batchedEntries) {
Iterator<LogEntry> iterator = batchedEntries.iterator();
while (iterator.hasNext()) {
if (!monitor.isCanceled()) {
LogEntry entry = iterator.next();
pushEntry(entry);
iterator.remove();
}
}
}
asyncRefresh(true);
Expand Down Expand Up @@ -1159,10 +1174,12 @@ private LogEntry createLogEntry(FrameworkLogEntry input) {
return logEntry;
}

private synchronized void pushEntry(LogEntry entry) {
if (LogReader.isLogged(entry, fMemento)) {
group(Collections.singletonList(entry));
limitEntriesCount();
private void pushEntry(LogEntry entry) {
synchronized (elements) {
if (LogReader.isLogged(entry, fMemento)) {
group(Collections.singletonList(entry));
limitEntriesCount();
}
}
asyncRefresh(true);
}
Expand Down Expand Up @@ -1558,6 +1575,7 @@ private void setComparator(byte sortType) {
date2 = ((LogSession) e2).getDate() == null ? 0 : ((LogSession) e2).getDate().getTime();
}
if (date1 == date2) {
// XXX: this breaks stable sort, as elements is mutable
int result = elements.indexOf(e2) - elements.indexOf(e1);
if (DATE_ORDER == DESCENDING)
result *= DESCENDING;
Expand Down Expand Up @@ -1653,6 +1671,8 @@ public int compare(Viewer viewer, Object e1, Object e2) {
// i.e. latest log message first, therefore index(e2)-index(e1)
result = indexOf(children, e2) - indexOf(children, e1);
} else {
// XXX: this breaks stable sort, as elements is
// mutable
result = elements.indexOf(e1) - elements.indexOf(e2);
}
if (DATE_ORDER == DESCENDING)
Expand Down