Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
88 commits
Select commit Hold shift + click to select a range
b8ccbe0
Revert "Revert changes supposed to be on a feature branch"
May 30, 2025
7422f4c
Reduce complexity of prover based shutdown getter
Jun 2, 2025
a43d183
Reduce complexity of prover based shutdown getter even more
Jun 2, 2025
5af69e6
Remove unnecessary comment in BitwuzlaTheoremProver
Jun 2, 2025
2b81ac8
Allow and use direct access to prover specific ShutdownNotifier
Jun 2, 2025
92f2f60
Add Int and BV test for context shutdown and prover reuse afterward
Jun 2, 2025
b048f56
Add default implementation of isUnsat() to centralize checks (closed,…
Jun 2, 2025
e73b889
Add more timeout/shutdown tests:
Jun 2, 2025
ba6c2bc
Add a model test that checks error when requesting a model for UNSAT …
Jun 2, 2025
28d5648
Remove unnecessary exceptions from test methods in TimeoutTest
Jun 2, 2025
5de5971
Format TimeoutTest
Jun 2, 2025
0f8e757
Add default handling for basic prover API like isUnsat(), getUnsatCor…
Jun 2, 2025
1b6f52b
Remove unnecessary final modifiers in AbstractProver
Jun 2, 2025
77ab3d6
Add more uniform handling of prerequisites of interpolation API
Jun 2, 2025
223624c
Add clarification JavaDoc for getShutdownManagerForProver()
Jun 2, 2025
55f8e08
Add clarification JavaDoc about needed options for unsatCoreOverAssum…
Jun 2, 2025
6613b21
Add more internal information about shutdown re-use for Z3 and Bitwuzla
Jun 2, 2025
843b0ee
Format JavaDoc
Jun 2, 2025
e6587bd
Add API for requesting new prover environments with a dedicated prove…
Jun 4, 2025
418bc8b
Remove old getter for created shutdown manager in the BasicProverEnvi…
Jun 4, 2025
c85042b
Add new prover environments with a dedicated prover shutdown notifier…
Jun 4, 2025
2fe5c2b
Add dedicated prover shutdown notifier to abstract provers
Jun 4, 2025
028ef5b
Add dedicated prover shutdown notifier to Bitwuzla
Jun 4, 2025
b122b50
Add dedicated prover shutdown notifier to ShutdownHook
Jun 4, 2025
b1ad38f
Add dedicated prover shutdown notifier to Boolector and remove unneed…
Jun 4, 2025
856d3b6
Add dedicated prover shutdown notifier to CVC4
Jun 4, 2025
3b16949
Add dedicated prover shutdown notifier to CVC5 implementation, but ma…
Jun 4, 2025
fa7dfb2
Add dedicated prover shutdown notifier to MathSAT5
Jun 4, 2025
37a58ee
Add dedicated prover shutdown notifier to OpenSMT2
Jun 4, 2025
3538fdc
Add dedicated prover shutdown notifier to Princess but make it not us…
Jun 4, 2025
9e6d0ba
Add dedicated prover shutdown notifier to SMTInterpol but make it not…
Jun 4, 2025
02bdda9
Add dedicated prover shutdown notifier to Yices2
Jun 4, 2025
30a74dd
Add dedicated prover shutdown notifier to Z3
Jun 4, 2025
1812dfc
Change TimeoutTest for the new way prover shutdown can be requested a…
Jun 4, 2025
17ec72e
Add default behavior/failure checks for getEvaluator()
Jun 4, 2025
5a20e6d
Add default behavior/failure checks for pop() for termination
Jun 11, 2025
0b95f11
Concat static string error msgs
Jun 11, 2025
1cd34dd
Add shutdown reason to exception msg for non-interrupted exception th…
Jun 11, 2025
c85c030
Add recovery of assertProverAPIThrowsInterruptedException from method…
Jun 11, 2025
407f52a
Remove redundant checks from BoolectorTheoremProver.java
Jun 11, 2025
384c7b1
Remove commented out old code
Jun 11, 2025
97c44b6
Add full shutdown/interrupt exception handling to Z3 model
Jun 11, 2025
cfa1cdb
Remove redundant checks and code from provers/solvers
Jun 11, 2025
799385c
Directly use shutdown exception text constant when checking for hidde…
Jun 25, 2025
b4fbc2f
Lazily eval shutdown msg when checking in non throwing methods
Jun 25, 2025
1f5c866
Reduce args for CVC5InterpolatingProver constructor
Jun 25, 2025
676aebb
Apply refaster patch
Jun 25, 2025
65407fb
Switch to lazy and self implemented shutdown state check
Jun 26, 2025
2e83b93
Centralize getInterpolant ID checking and also give back the mismatch…
Jun 26, 2025
ee5eee0
Centralize getInterpolant ID checking and also give back the mismatch…
Jun 26, 2025
c1dc784
Correctly reset assumptions for interpolation in the assumption wrapp…
Jun 26, 2025
41daa1f
Remove unneeded lazy handling of error msg
Jun 26, 2025
d06d41c
Extend TimeoutTest with checking for all prover API, including interp…
Jun 26, 2025
4a4cac8
Improve interpolation input error messages by including a null check …
Jun 30, 2025
0c683e8
Reduce query size for timeout test (was unnecessarily high)
Jun 30, 2025
79ba7d7
Rename shutdownManagers in tests to make their use/source more clear
Jun 30, 2025
295e01a
Make internal prover variables private and add setter and getter wher…
Jun 30, 2025
a9023f9
Remove unneeded lambda from Mathsat termination check
Jun 30, 2025
3ca9316
Add a sanity check in DebuggingBasicProverEnvironment for getModelAss…
Jun 30, 2025
9e63816
Mark implementation specifications with @implNote in SolverContext
Jun 30, 2025
ac5be00
Make JavaDoc clearer for SolverContext prover requests and their inte…
Jun 30, 2025
1d9ec5a
Improve JavaDoc of SolverContext in regard to shutdown of provers
Jun 30, 2025
e4bf4e8
Hide common Strings used in provers from users
Jun 30, 2025
f16f77d
Make common Strings used in provers static
Jun 30, 2025
1ab6955
Re-add default impl of getModelAssignments()
Jun 30, 2025
fc323b9
Re-throw exception when clearing assumptions, and it's not another ex…
Jul 1, 2025
1b268a3
Improve JavaDoc of constructor methods of new prover environments wit…
Jul 21, 2025
c4a7a65
Add warning for sneaky SolverExceptions in Model JavaDoc
Jul 29, 2025
b7da958
Throw sneaky SolverExceptions in Mathsat Model instead of confusing I…
Jul 29, 2025
50094eb
Add more sneaky throw warnings to API that may throw a SolverExceptio…
Jul 29, 2025
89837d5
Remove sneaky throw of SolverException from Mathsat native call
Jul 30, 2025
dbbdc31
Remove sneaky throw of SolverException from Z3 model and add Interrup…
Jul 30, 2025
b0e7b44
Add SolverException and InterruptedException to many calls related to…
Jul 30, 2025
0ca8d4f
Remove sneaky throws for Z3 specific code entirely
Jul 30, 2025
cd51b02
Add Solver- and InterruptedException to push() and addConstraint() API
Jul 30, 2025
898832d
Remove warnings about sneaky throws where they can't happen anymore
Jul 30, 2025
c900399
Format: remove warnings about sneaky throws where they can't happen a…
Jul 30, 2025
2166410
Rename exception variable to fit naming schema
Jul 30, 2025
64b21b3
Improve warning for sneaky throws in model iterator() as suggested by…
Jul 30, 2025
cd3ef20
Merge feature branch 481-mathsat5-returns-null-for-msat_model_create_…
Jul 31, 2025
477efd0
Remove special handling of interrupts for API that did not throw Inte…
Jul 31, 2025
ca14eff
Remove special handling of interrupts for API that did not throw Inte…
Jul 31, 2025
4b429db
Switch all expected exceptions after interrupt to InterruptedExceptio…
Jul 31, 2025
d447902
Remove toString() in lazy evaluation to be actually lazy in AbstractP…
Jul 31, 2025
22bc811
Move JavaDoc from throws to general info for prover environment creat…
Jul 31, 2025
a75ec8a
Use dummy ShutdownNotifier instead of null if there is no prover spec…
Jul 31, 2025
5f30d42
Remove null checks for prover based ShutdownNotifier when no longer n…
Aug 4, 2025
b8c9a06
Remove Z3 specific code for throwing checked exceptions as runtime ex…
Aug 4, 2025
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
22 changes: 22 additions & 0 deletions src/org/sosy_lab/java_smt/api/BasicProverEnvironment.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import java.util.List;
import java.util.Optional;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.sosy_lab.common.ShutdownManager;
import org.sosy_lab.common.ShutdownNotifier;
import org.sosy_lab.java_smt.api.SolverContext.ProverOptions;

/**
Expand Down Expand Up @@ -202,4 +204,24 @@ interface AllSatCallback<R> {
default boolean registerUserPropagator(UserPropagator propagator) {
return false;
}

/**
* Returns the {@link ShutdownManager} registered for this {@link ProverEnvironment}. It is
* guaranteed to be a child of the {@link ShutdownNotifier} given to the creating {@link
* SolverContext}, resulting in shutdown when the {@link SolverContext}s notifier is shutting
* down. The notifier returned here can be shut down independently of the creating contexts
* notifier.
*
* @return a {@link ShutdownManager} who is the child of the {@link ShutdownNotifier} used in the
* creating {@link SolverContext}, that can be used to shut down only this {@link
* ProverEnvironment}.
* @throws UnsupportedOperationException if the solver does not support prover specific shutdown.
*/
default ShutdownManager getShutdownManagerForProver() {
// Override this with the prover specific ShutdownManagers notifier for supporting solvers.
// The solver should then use the prover specific ShutdownManagers notifier for stopping
// instead of the contexts' notifier!
throw new UnsupportedOperationException(
"The chosen solver does not support isolated prover shutdown");
}
}
11 changes: 10 additions & 1 deletion src/org/sosy_lab/java_smt/basicimpl/AbstractProver.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import java.util.List;
import java.util.Set;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.sosy_lab.common.ShutdownManager;
import org.sosy_lab.common.ShutdownNotifier;
import org.sosy_lab.java_smt.api.BasicProverEnvironment;
import org.sosy_lab.java_smt.api.BooleanFormula;
import org.sosy_lab.java_smt.api.Evaluator;
Expand All @@ -47,7 +49,11 @@ public abstract class AbstractProver<T> implements BasicProverEnvironment<T> {

private static final String TEMPLATE = "Please set the prover option %s.";

protected AbstractProver(Set<ProverOptions> pOptions) {
// Do we even need this?
private final ShutdownNotifier contextShutdownNotifier;
protected final ShutdownManager proverShutdownManager;

protected AbstractProver(ShutdownNotifier pShutdownNotifier, Set<ProverOptions> pOptions) {
generateModels = pOptions.contains(ProverOptions.GENERATE_MODELS);
generateAllSat = pOptions.contains(ProverOptions.GENERATE_ALL_SAT);
generateUnsatCores = pOptions.contains(ProverOptions.GENERATE_UNSAT_CORE);
Expand All @@ -56,6 +62,9 @@ protected AbstractProver(Set<ProverOptions> pOptions) {
enableSL = pOptions.contains(ProverOptions.ENABLE_SEPARATION_LOGIC);

assertedFormulas.add(LinkedHashMultimap.create());

contextShutdownNotifier = pShutdownNotifier;
proverShutdownManager = ShutdownManager.createWithParent(contextShutdownNotifier);
}

protected final void checkGenerateModels() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,14 @@
*/
public abstract class AbstractProverWithAllSat<T> extends AbstractProver<T> {

protected final ShutdownNotifier shutdownNotifier;
private final BooleanFormulaManager bmgr;

protected AbstractProverWithAllSat(
Set<ProverOptions> pOptions,
BooleanFormulaManager pBmgr,
ShutdownNotifier pShutdownNotifier) {
super(pOptions);
super(pShutdownNotifier, pOptions);
bmgr = pBmgr;
shutdownNotifier = pShutdownNotifier;
}

@Override
Expand Down Expand Up @@ -68,7 +66,7 @@ private <R> void iterateOverAllModels(
AllSatCallback<R> callback, List<BooleanFormula> importantPredicates)
throws SolverException, InterruptedException {
while (!isUnsat()) {
shutdownNotifier.shutdownIfNecessary();
proverShutdownManager.getNotifier().shutdownIfNecessary();

ImmutableList.Builder<BooleanFormula> valuesOfModel = ImmutableList.builder();
try (Evaluator evaluator = getEvaluatorWithoutChecks()) {
Expand All @@ -88,11 +86,11 @@ private <R> void iterateOverAllModels(

final ImmutableList<BooleanFormula> values = valuesOfModel.build();
callback.apply(values);
shutdownNotifier.shutdownIfNecessary();
proverShutdownManager.getNotifier().shutdownIfNecessary();

BooleanFormula negatedModel = bmgr.not(bmgr.and(values));
addConstraint(negatedModel);
shutdownNotifier.shutdownIfNecessary();
proverShutdownManager.getNotifier().shutdownIfNecessary();
}
}

Expand All @@ -113,7 +111,7 @@ private <R> void iterateOverAllPredicateCombinations(
Deque<BooleanFormula> valuesOfModel)
throws SolverException, InterruptedException {

shutdownNotifier.shutdownIfNecessary();
proverShutdownManager.getNotifier().shutdownIfNecessary();

if (isUnsat()) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import org.sosy_lab.common.ShutdownManager;
import org.sosy_lab.java_smt.api.BasicProverEnvironment;
import org.sosy_lab.java_smt.api.BooleanFormula;
import org.sosy_lab.java_smt.api.Model;
Expand Down Expand Up @@ -128,4 +129,9 @@ public <R> R allSat(AllSatCallback<R> pCallback, List<BooleanFormula> pImportant
clearAssumptions();
return delegate.allSat(pCallback, pImportant);
}

@Override
public ShutdownManager getShutdownManagerForProver() throws UnsupportedOperationException {
return delegate.getShutdownManagerForProver();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.List;
import java.util.Optional;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.sosy_lab.common.ShutdownManager;
import org.sosy_lab.java_smt.api.BasicProverEnvironment;
import org.sosy_lab.java_smt.api.BooleanFormula;
import org.sosy_lab.java_smt.api.Model;
Expand All @@ -29,6 +30,11 @@ class DebuggingBasicProverEnvironment<T> implements BasicProverEnvironment<T> {
debugging = pDebugging;
}

@Override
public ShutdownManager getShutdownManagerForProver() throws UnsupportedOperationException {
return delegate.getShutdownManagerForProver();
}

@Override
public void pop() {
debugging.assertThreadLocal();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
package org.sosy_lab.java_smt.delegate.debugging;

import java.util.Optional;
import org.sosy_lab.common.ShutdownManager;
import org.sosy_lab.common.rationals.Rational;
import org.sosy_lab.java_smt.api.Formula;
import org.sosy_lab.java_smt.api.OptimizationProverEnvironment;
Expand All @@ -26,6 +27,11 @@ public DebuggingOptimizationProverEnvironment(
debugging = pDebugging;
}

@Override
public ShutdownManager getShutdownManagerForProver() throws UnsupportedOperationException {
return delegate.getShutdownManagerForProver();
}

@Override
public int maximize(Formula objective) {
debugging.assertThreadLocal();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.Optional;
import java.util.logging.Level;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.sosy_lab.common.ShutdownManager;
import org.sosy_lab.common.log.LogManager;
import org.sosy_lab.java_smt.api.BasicProverEnvironment;
import org.sosy_lab.java_smt.api.BooleanFormula;
Expand All @@ -38,6 +39,11 @@ class LoggingBasicProverEnvironment<T> implements BasicProverEnvironment<T> {
logger = checkNotNull(pLogger);
}

@Override
public ShutdownManager getShutdownManagerForProver() throws UnsupportedOperationException {
return wrapped.getShutdownManagerForProver();
}

@Override
public @Nullable T push(BooleanFormula f) throws InterruptedException {
logger.log(Level.FINE, "up to level " + level++);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.List;
import java.util.Optional;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.sosy_lab.common.ShutdownManager;
import org.sosy_lab.java_smt.api.BasicProverEnvironment;
import org.sosy_lab.java_smt.api.BooleanFormula;
import org.sosy_lab.java_smt.api.Model;
Expand All @@ -35,6 +36,11 @@ class StatisticsBasicProverEnvironment<T> implements BasicProverEnvironment<T> {
stats.provers.getAndIncrement();
}

@Override
public ShutdownManager getShutdownManagerForProver() throws UnsupportedOperationException {
return delegate.getShutdownManagerForProver();
}

@Override
public void pop() {
stats.pop.getAndIncrement();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.util.List;
import java.util.Optional;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.sosy_lab.common.ShutdownManager;
import org.sosy_lab.java_smt.api.BasicProverEnvironment;
import org.sosy_lab.java_smt.api.BooleanFormula;
import org.sosy_lab.java_smt.api.Model;
Expand All @@ -31,6 +32,11 @@ class SynchronizedBasicProverEnvironment<T> implements BasicProverEnvironment<T>
sync = checkNotNull(pSync);
}

@Override
public ShutdownManager getShutdownManagerForProver() throws UnsupportedOperationException {
return delegate.getShutdownManagerForProver();
}

@Override
public void pop() {
synchronized (sync) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.List;
import java.util.Optional;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.sosy_lab.common.ShutdownManager;
import org.sosy_lab.java_smt.api.BasicProverEnvironment;
import org.sosy_lab.java_smt.api.BooleanFormula;
import org.sosy_lab.java_smt.api.FormulaManager;
Expand Down Expand Up @@ -144,6 +145,11 @@ public <R> R allSat(AllSatCallback<R> pCallback, List<BooleanFormula> pImportant
}
}

@Override
public ShutdownManager getShutdownManagerForProver() throws UnsupportedOperationException {
return delegate.getShutdownManagerForProver();
}

private class AllSatCallbackWithContext<R> implements AllSatCallback<R> {

private final AllSatCallback<R> delegateCallback;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import java.util.Collection;
import java.util.List;
import org.sosy_lab.common.ShutdownManager;
import org.sosy_lab.java_smt.api.BooleanFormula;
import org.sosy_lab.java_smt.api.FormulaManager;
import org.sosy_lab.java_smt.api.InterpolatingProverEnvironment;
Expand All @@ -33,6 +34,11 @@ class SynchronizedInterpolatingProverEnvironmentWithContext<T>
delegate = checkNotNull(pDelegate);
}

@Override
public ShutdownManager getShutdownManagerForProver() throws UnsupportedOperationException {
return delegate.getShutdownManagerForProver();
}

@Override
public BooleanFormula getInterpolant(Collection<T> pFormulasOfA)
throws SolverException, InterruptedException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.util.Optional;
import java.util.Set;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.sosy_lab.common.ShutdownManager;
import org.sosy_lab.common.ShutdownNotifier;
import org.sosy_lab.java_smt.api.BooleanFormula;
import org.sosy_lab.java_smt.api.Model;
Expand All @@ -38,7 +39,10 @@ class BitwuzlaTheoremProver extends AbstractProverWithAllSat<Void> implements Pr
new Terminator() {
@Override
public boolean terminate() {
return shutdownNotifier.shouldShutdown(); // shutdownNotifer is defined in the superclass
return proverShutdownManager
.getNotifier()
.shouldShutdown(); // shutdownNotifer is defined in the
// superclass
Copy link
Member

Choose a reason for hiding this comment

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

The comment is formatted ugly, and it might not be understandable, please rephrase.

Is getNotifier() a "fast" method call or does it construct any objects? The callback is expected to be executed quite often from native solver code. We could store the notifier directly within the class and just reuse it.

Addtiionally, we already have a method for getting the manager. Why do we then use direct access here? Please use the getter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the comment, as the new default should be obvious.

getNotifier() does not construct any objects, it just returns the notifier that is created when the ShutdownManager is created. You can argue that it is faster to have the reference directly, but this is built once when the prover is created. I changed it so that the notifier is generally accessible directly in the superclass.

Thanks for the feedback!

}
};
private final Bitwuzla env;
Expand Down Expand Up @@ -119,7 +123,8 @@ private boolean readSATResult(Result resultValue) throws SolverException, Interr
return false;
} else if (resultValue == Result.UNSAT) {
return true;
} else if (resultValue == Result.UNKNOWN && shutdownNotifier.shouldShutdown()) {
} else if (resultValue == Result.UNKNOWN
&& proverShutdownManager.getNotifier().shouldShutdown()) {
Copy link
Member

Choose a reason for hiding this comment

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

Is it guaranteed that the manager always returns the same notifier-instance and the notifier always returns the same flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! The notifier is created final and only once, see here: private final ShutdownNotifier notifier = new ShutdownNotifier(this); The flag can not be changed once a shutdown is triggered.

Copy link
Member

Choose a reason for hiding this comment

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

Referencing some existing source code is no "guarantee".
The guarantee would be a JavaDoc specification for the ShutdownManager-API, which is missing:
https://sosy-lab.github.io/java-common-lib/api/org/sosy_lab/common/ShutdownManager.html#getNotifier()

A user can provide his own implementation of the shutdown-manager and change its behaviour.
I really would prefer to keep a local reference to this notifier.

Copy link
Member

Choose a reason for hiding this comment

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

There is no intention of a guarantee that getNotifier() always returns the same instance, but there is definitively the intention that this does not matter and that all notifiers handed out by a manager always return the shutdown state of the manager, and that shouldShutdown() will keep returning true forever after it has returned true once.

Since the split in separate manager and notifier classes this was not fully written down in the JavaDoc, but I improved this now: sosy-lab/java-common-lib@108faa6

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@PhilippWendler i just noticed that there is/can be a delay in between the shutdown being requested and the children being informed. See the 2 tests here. Is this intentional?

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure what exactly you are referring to. But if you have multi-threaded code, then you can of course never be sure when each of the threads is scheduled in relation to the others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but i would have expected that the chain of signals in the shutdown notifiers/managers is built in such a way that they are all informed in a synchronized way.

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean with "synchronized way"? "synchronized" is not some magic global property, one can only synchronize against specific things.

If you want help, you need to be more specific. What is the sequence of operations that you are observing and that is unexpected/unwanted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was merely curious ;D
We discussed this offline sufficiently. Thank you!

throw new InterruptedException();
} else {
throw new SolverException("Bitwuzla returned UNKNOWN.");
Expand Down Expand Up @@ -246,6 +251,11 @@ protected BitwuzlaModel getEvaluatorWithoutChecks() {
Collections2.transform(getAssertedFormulas(), creator::extractInfo)));
}

@Override
public ShutdownManager getShutdownManagerForProver() throws UnsupportedOperationException {
return proverShutdownManager;
}
Copy link
Member

Choose a reason for hiding this comment

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

One could move this method higher up in the hierarchy and just provide it in the abstract super-class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it completely, as we didn't really need it.


public boolean isClosed() {
return closed;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ protected BoolectorAbstractProver(
this.manager = manager;
this.creator = creator;
this.btor = btor;
terminationCallback = shutdownNotifier::shouldShutdown;
terminationCallback = proverShutdownManager.getNotifier()::shouldShutdown;
Copy link
Member

Choose a reason for hiding this comment

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

Just for comparison to above: Here, we keep the same notifier and reuse it for all callback-calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We build a new manager in every prover creation (see here) and then use this new managers notifier to request shutdowns. So we always use the one notifier bound to the one manager of the prover (that is a child of the contexts ShutdownNotifier)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is now outdated due to the changes.

terminationCallbackHelper = addTerminationCallback();

isAnyStackAlive = pIsAnyStackAlive;
Expand Down
13 changes: 10 additions & 3 deletions src/org/sosy_lab/java_smt/solvers/cvc4/CVC4TheoremProver.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Optional;
import java.util.Set;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.sosy_lab.common.ShutdownManager;
import org.sosy_lab.common.ShutdownNotifier;
import org.sosy_lab.java_smt.api.BasicProverEnvironment;
import org.sosy_lab.java_smt.api.BooleanFormula;
Expand Down Expand Up @@ -202,11 +203,12 @@ public boolean isUnsat() throws InterruptedException, SolverException {
}

Result result;
try (ShutdownHook hook = new ShutdownHook(shutdownNotifier, smtEngine::interrupt)) {
shutdownNotifier.shutdownIfNecessary();
try (ShutdownHook hook =
new ShutdownHook(proverShutdownManager.getNotifier(), smtEngine::interrupt)) {
proverShutdownManager.getNotifier().shutdownIfNecessary();
result = smtEngine.checkSat();
}
shutdownNotifier.shutdownIfNecessary();
proverShutdownManager.getNotifier().shutdownIfNecessary();
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to avoid calling getNotifier() more than once here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just forgot to push everything. Should be removed now. Sorry for the delay.

return convertSatResult(result);
}

Expand Down Expand Up @@ -260,4 +262,9 @@ public void close() {
}
super.close();
}

@Override
public ShutdownManager getShutdownManagerForProver() throws UnsupportedOperationException {
return proverShutdownManager;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public boolean isUnsat() throws InterruptedException, SolverException {

/* Shutdown currently not possible in CVC5. */
Result result = solver.checkSat();
shutdownNotifier.shutdownIfNecessary();
proverShutdownManager.getNotifier().shutdownIfNecessary();
return convertSatResult(result);
}

Expand Down Expand Up @@ -251,4 +251,10 @@ public void close() {
}
super.close();
}

/* TODO: revisit once CVC5 supports interruption
@Override
protected ShutdownManager getShutdownManagerForProverImpl() throws UnsupportedOperationException {
return proverShutdownManager;
}*/
}
Loading