Skip to content

Conversation

@puyral
Copy link

@puyral puyral commented Apr 8, 2025

This adds bindings and an API for the user propagator and the on_clause callback.

I would consider this still experimental, as I'm not fully sure of what z3 might do with my objects (e.g., weird lifetimes and thread safety).

This also closes #343

puyral added 7 commits April 4, 2025 09:29
ignores Macos' DS_Store and some `nix` related files
Not tested yet, so there are probably many typos around still
many things are not ready yet, notably I'm not sure of the lifetimes of
things. I skipped fresh entirely
Some things can likely still be improved, but this is a good
initial implementation
This way there is no need for handles, it also makes it easier to add
the `on_clause` API
@puyral
Copy link
Author

puyral commented Apr 8, 2025

So apparently Ubuntu ships with a version of z3 that is too old for the user propagator...

The previous version half assumed there was no multithreading (no
requierement for Sync), half assumed there was (it was using `ref`
everywhere). This commit fully goes for the no multithreading version.
- &mut where it makes sense
- the UPSolver now owns the UserPropagators, to prevent potential
  aliasings
@toolCHAINZ toolCHAINZ added the enhancement New feature or request label Jul 15, 2025
@puyral puyral changed the title Add bindings to the user propagator WIP: Add bindings to the user propagator Jul 17, 2025
@puyral puyral marked this pull request as draft July 17, 2025 08:22
@puyral
Copy link
Author

puyral commented Jul 17, 2025

So I discovered that we can get null pointer back from z3 with my implementation of fresh_eh (which will result in a crash).

TL;DR: current lifetimes don't match z3's and I don't yet have a solution to make them agree with Rust's borrow checker assumptions.

Currently, it actually returns the null pointer that z3 gives back. This was done on purpose because implementing this function causes me huge lifetimes problems.

To give an idea of the problem. The C++ implementation for fresh says:

user_propagators created using fresh() are created during search, and their lifetimes are restricted to search time. They should be garbage collected by the propagator used to invoke fresh(). The life-time of the Z3_context object can only be assumed valid during callbacks, such as fixed(), which contains expressions based on the context.

In practice, z3 gives a new Z3_context that the inner solver will use. I understand that this context may get garbage collected before we reach the end of the search. So, no z3 object should "escape" the user propagator.
To add to the problem, it's our responsibility to garbage collect the subsolver's user propagator. We don't know when we should drop it, aside from the fact that it should happen before the very end of the search. At that point, that user propagator is almost sure to hold invalid pointers/references (e.g., the Context) that might cause UB during Drop.

These problems tend to cascade effects to the rest of the user propagator API.

I'm switching this into a draft since I don't have an immediate solution.

If you are using z3 in a way that won't spawn subsolvers, then you are safe to use the current API.

@toolCHAINZ
Copy link
Member

toolCHAINZ commented Aug 29, 2025

It may be worth revisiting this in light of #401, #404, and #417.

Now that contexts are refcounted and implicit, there shouldn't be an issue dealing with arcane lifetimes. The trick would be designing the API to ensure correctness and memory safety in the face of callbacks that use Contexts we do not control.

I haven't done a detailed read of the contribution you have here, but going from your description of the difficulty, there's a general approach that I think could work:

  • Change ContextInternal to an enum, with an internal "owned" variant (what it has right now) and a "borrowed" variant (which can be unsafely created from a raw Z3_Context). The "owned" variant would behave as it does now, destroying the inner context on Drop. The "borrowed" variant would not, making it suitable for cases like this where we are given a Context that Z3 itself is managing.
  • Any Rust code running in this user propagator context would have to be expressed in a Send + Sync closure. There would be a wrapper that receives the Z3-managed Context, builds a "borrowed" Context out of it, and runs the closure using that Context in Context::thread_local.

This would ensure that no Ast or Solver associated with that Context can escape, and if existing stuff outside the propagator is needed for these callbacks, they can be moved into the closure with synchronized()/recover().

@toolCHAINZ toolCHAINZ added the blocked: apt z3 Features that we can merge after the next Ubuntu LTS bumps the ancient z3 version label Aug 29, 2025
@puyral
Copy link
Author

puyral commented Sep 18, 2025

Amazing! It sounds like this does indeed solve most of my issues!

I'll have a try at this Weekend

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked: apt z3 Features that we can merge after the next Ubuntu LTS bumps the ancient z3 version enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Retrieve non-unsat proof attempts via on_clause callback

2 participants