From 392d4a0b29821a051f9cabac30e71b6666a993b0 Mon Sep 17 00:00:00 2001 From: Gunnar Wagenknecht Date: Sun, 23 Jul 2023 17:46:26 +0200 Subject: [PATCH] Ensure there is no duplicate opening activity --- .../eclipse/core/model/BazelElement.java | 17 +-- .../core/model/BazelElementOpenJob.java | 129 ++++++++++++++++++ 2 files changed, 138 insertions(+), 8 deletions(-) create mode 100644 bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelElementOpenJob.java diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelElement.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelElement.java index 438f560f..f86c9805 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelElement.java +++ b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelElement.java @@ -13,11 +13,11 @@ */ package com.salesforce.bazel.eclipse.core.model; -import static java.lang.String.format; import static java.util.Objects.requireNonNull; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IPath; +import org.eclipse.core.runtime.OperationCanceledException; import com.salesforce.bazel.eclipse.core.model.cache.BazelElementInfoCache; import com.salesforce.bazel.sdk.model.BazelLabel; @@ -131,13 +131,14 @@ protected final I getInfo() throws CoreException { getParent().getInfo(); } - // loads can be potentially expensive; we tolerate this here and may create multiple infos - info = requireNonNull( - createInfo(), - () -> format("invalid implementation of #createInfo in %s; must not return null!", this.getClass())); - - // however, we ensure there is at most one info in the cache and this is what we use - return infoCache.putOrGetCached(this, info); + // loads can be potentially expensive; we synchronize on the location + var location = getLocation(); + var openJob = new BazelElementOpenJob<>(location != null ? location : IPath.ROOT, this, infoCache); + try { + return openJob.open(); + } catch (InterruptedException e) { + throw new OperationCanceledException("Interrupted while opening element."); + } } BazelElementInfoCache getInfoCache() { diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelElementOpenJob.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelElementOpenJob.java new file mode 100644 index 00000000..cdef5cb4 --- /dev/null +++ b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelElementOpenJob.java @@ -0,0 +1,129 @@ +/*- + * + */ +package com.salesforce.bazel.eclipse.core.model; + +import static java.lang.String.format; +import static java.util.Objects.requireNonNull; + +import java.util.concurrent.CountDownLatch; + +import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IPath; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.OperationCanceledException; +import org.eclipse.core.runtime.Status; +import org.eclipse.core.runtime.jobs.ISchedulingRule; +import org.eclipse.core.runtime.jobs.Job; + +import com.salesforce.bazel.eclipse.core.model.cache.BazelElementInfoCache; + +/** + * A special job for opening a {@link BazelElement} preventing concurrent load attempts. + *

+ * The job can only be used once! + *

+ */ +class BazelElementOpenJob extends Job implements ISchedulingRule { + + final IPath location; + final BazelElement bazelElement; + final CountDownLatch opened = new CountDownLatch(1); + private volatile I info; + private volatile CoreException openException; + private volatile OperationCanceledException cancelException; + private final BazelElementInfoCache infoCache; + + BazelElementOpenJob(IPath location, BazelElement bazelElement, BazelElementInfoCache infoCache) { + super(format("Opening '%s'...", location.toOSString())); + this.location = location; + this.bazelElement = bazelElement; + this.infoCache = infoCache; + + setSystem(true); + setUser(false); + setRule(this); + setPriority(SHORT); + } + + @Override + public boolean contains(ISchedulingRule rule) { + if (rule == this) { + return true; + } + + if (rule instanceof BazelElementOpenJob otherJob) { + var otherLocation = otherJob.location; + return location.isPrefixOf(otherLocation); + } + + return false; + } + + @Override + public boolean isConflicting(ISchedulingRule rule) { + if (rule == this) { + return true; + } + + if (rule instanceof BazelElementOpenJob otherJob) { + var otherLocation = otherJob.location; + return location.isPrefixOf(otherLocation) || otherLocation.isPrefixOf(location); + } + + return false; + } + + public I open() throws InterruptedException, CoreException { + if (opened.getCount() != 1) { + throw new IllegalStateException( + "Attempt to call the same open job twice. This is a programming error! Change the code."); + } + + schedule(); + opened.await(); + if (cancelException != null) { + throw new OperationCanceledException("Opening canceled"); + } + if (openException != null) { + throw new CoreException( + Status.error( + format("Error opening element at '%s': %s", location, openException.getMessage()), + openException)); + } + + return requireNonNull(info, "Programming error: the info is expected to be set at this point. Check the code!"); + } + + @Override + protected IStatus run(IProgressMonitor monitor) { + try { + // check check within scheduling rule to ensure we don't load twice + info = infoCache.getIfPresent(bazelElement); + if (info != null) { + return Status.OK_STATUS; + } + + // load + info = requireNonNull( + bazelElement.createInfo(), + () -> format( + "invalid implementation of #createInfo in %s; must not return null!", + bazelElement.getClass())); + + // store in cache + infoCache.putOrGetCached(bazelElement, info); + } catch (OperationCanceledException e) { + cancelException = e; + return Status.CANCEL_STATUS; + } catch (CoreException e) { + openException = e; + return Status.error(format("Opening element at '%s' failed with: '%s'", location, e.getMessage()), e); + } finally { + opened.countDown(); + } + + return Status.OK_STATUS; + } +}