scipopt/russcip

segfault (use after free) when accessing solution after model is dropped

Nudelmeister opened this issue · 3 comments

use russcip::prelude::*;

fn main() {
    // Create model
    let mut model = Model::new()
        .hide_output()
        .include_default_plugins()
        .create_prob("test")
        .set_obj_sense(ObjSense::Maximize);

    // Add variables
    let x1 = model.add_var(0., f64::INFINITY, 3., "x1", VarType::Integer);
    let x2 = model.add_var(0., f64::INFINITY, 4., "x2", VarType::Integer);

    // Add constraints
    model.add_cons(
        vec![x1.clone(), x2.clone()],
        &[2., 1.],
        -f64::INFINITY,
        100.,
        "c1",
    );
    model.add_cons(
        vec![x1.clone(), x2.clone()],
        &[1., 2.],
        -f64::INFINITY,
        80.,
        "c2",
    );

    let sol = model.solve().best_sol().unwrap(); // Temporary value returned from `model.solve()` is dropped.

    dbg!(sol); // <-- Segfault here
}

From looking over the russcip code the cause seems obvious.
ScipPtr's drop impl is responsible for cleaning up and freeing, but Solution also holds the same raw pointer to Scip, causing a use after free.

My first thought for a fix would be to remove the pointer aliasing, only ScipPtr should hold the pointer to Scip. ScipPtr could then be reference counted. Alternatively, change the API so that it more closely mimics that of scip and require passing the ScipPtr separately.

Thanks a lot for the report and the suggested fix! I think I like the solution of making ScipPtr count the given references as it gives more freedom for russcip and would allow for more ergonomic use. I will work on it as soon as I can :)

A (zero-cost) alternative is to add a lifetime parameter. That would be API breaking and less ergonomic, but variable lifetimes seems to be the root cause of the problem.

So the signature of Solution would become:

Solution<'a>

But I might be missing some intricacies of the libreary

Fixed in #149