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

MT hardening FileInfoReader #568 #569

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
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: %pluginName
Bundle-SymbolicName: org.eclipse.equinox.p2.transport.ecf
Bundle-Version: 1.4.300.qualifier
Bundle-Version: 1.4.400.qualifier
Bundle-RequiredExecutionEnvironment: JavaSE-17
Require-Bundle: org.eclipse.ecf;bundle-version="3.1.0",
org.eclipse.ecf.filetransfer;bundle-version="4.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.io.FileNotFoundException;
import java.io.IOException;
import java.net.URI;
import java.util.concurrent.atomic.AtomicBoolean;
import org.eclipse.core.runtime.*;
import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.ecf.core.*;
Expand All @@ -34,21 +35,21 @@
* ECF). If such support is added, this class is easily modified.
*/
public class FileInfoReader extends Job implements IRemoteFileSystemListener {
private Exception exception;
private IProgressMonitor theMonitor;
private volatile Exception exception;
private volatile IProgressMonitor theMonitor;
private final int connectionRetryCount;
private final long connectionRetryDelay;
private final IConnectContext connectContext;
final Boolean[] barrier = new Boolean[1];
private IRemoteFile[] remoteFiles;
private IRemoteFileSystemRequest browseRequest;
private final AtomicBoolean barrierReached = new AtomicBoolean();
private volatile IRemoteFile[] remoteFiles;
private volatile IRemoteFileSystemRequest browseRequest;

@Override
protected IStatus run(IProgressMonitor monitor) {
synchronized (barrier) {
while (barrier[0] == null) {
synchronized (barrierReached) {
while (!barrierReached.get()) {
Copy link
Member

Choose a reason for hiding this comment

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

This changes behavior before any value TRUE/FALSE would have ended the loop now it loops forever if set to false.

try {
barrier.wait(1000);
barrierReached.wait(1000);
if (theMonitor.isCanceled() && browseRequest != null)
browseRequest.cancel();
} catch (InterruptedException e) {
Expand All @@ -60,14 +61,13 @@ protected IStatus run(IProgressMonitor monitor) {
}

/**
* Waits until request is processed (barrier[0] is non null).
* This is a bit of a hack, as it would be better if the ECFBrowser worked in similar fashion to
* file transfer were a custom job can be supplied.
* TODO: log an issue for ECF.
* Waits until request is processed (barrierReached.get()). This is a bit of a
* hack, as it would be better if the ECFBrowser worked in similar fashion to
* file transfer were a custom job can be supplied. TODO: log an issue for ECF.
*/
private void waitOnSelf() {
schedule();
while (barrier[0] == null) {
while (!barrierReached.get()) {
Copy link
Member

Choose a reason for hiding this comment

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

I must confess that I can't get what is the purpose of this loop... if I schedule a job once how can I join it more than once?

@merks any clue whats the purpose of this?

boolean logged = false;
try {
join();
Expand All @@ -84,7 +84,6 @@ private void waitOnSelf() {
*/
public FileInfoReader(IConnectContext aConnectContext) {
super(Messages.repo_loading); // job label - TODO: this is a bad label
barrier[0] = null;
// Hide this job.
setSystem(true);
setUser(false);
Expand Down Expand Up @@ -136,23 +135,24 @@ public long getLastModified(URI location, IProgressMonitor monitor) throws Authe
public void handleRemoteFileEvent(IRemoteFileSystemEvent event) {
exception = event.getException();
if (exception != null) {
synchronized (barrier) {
barrier[0] = Boolean.TRUE;
barrier.notify();
synchronized (barrierReached) {
barrierReached.set(true);
barrierReached.notifyAll();
}
} else if (event instanceof IRemoteFileSystemBrowseEvent) {
IRemoteFileSystemBrowseEvent fsbe = (IRemoteFileSystemBrowseEvent) event;
remoteFiles = fsbe.getRemoteFiles();
if (theMonitor != null)
theMonitor.worked(1);
synchronized (barrier) {
barrier[0] = Boolean.TRUE;
barrier.notify();
synchronized (barrierReached) {
barrierReached.set(true);
barrierReached.notifyAll();
}
} else {
synchronized (barrier) {
barrier[0] = Boolean.FALSE; // ended by unknown reason
barrier.notify();
synchronized (barrierReached) {
// ended by unknown reason, but ended
barrierReached.set(true);
barrierReached.notifyAll();
}
}
}
Expand Down
Loading