From ffbb200732887a6dbdbdf79d8ccf20da51c79456 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Beno=C3=AEt=20Legat?= <benoit.legat@gmail.com>
Date: Mon, 2 Jan 2017 17:46:49 +0100
Subject: [PATCH] fix multiple loadproblem! calls

---
 src/CplexSolverInterface.jl |  4 +++-
 src/cpx_env.jl              | 33 ++++++++++++++++++++++++++++++++-
 src/cpx_model.jl            | 34 ++++++----------------------------
 test/env.jl                 | 14 ++++++++++++++
 test/runtests.jl            |  1 +
 5 files changed, 56 insertions(+), 30 deletions(-)
 create mode 100644 test/env.jl

diff --git a/src/CplexSolverInterface.jl b/src/CplexSolverInterface.jl
index e5cc2b5bb6e..7cad105d8b6 100644
--- a/src/CplexSolverInterface.jl
+++ b/src/CplexSolverInterface.jl
@@ -20,7 +20,7 @@ function CplexMathProgModel(;options...)
         set_param!(env, string(name), value)
     end
 
-    m = CplexMathProgModel(_Model(env), nothing, nothing, nothing, nothing, nothing, nothing, NaN)
+    m = CplexMathProgModel(Model(env), nothing, nothing, nothing, nothing, nothing, nothing, NaN)
     return m
 end
 
@@ -76,6 +76,8 @@ function loadproblem!(m::CplexMathProgModel, filename::String)
 end
 
 function loadproblem!(m::CplexMathProgModel, A, collb, colub, obj, rowlb, rowub, sense)
+  # throw away old model but keep env
+  m.inner = Model(m.inner.env)
   add_vars!(m.inner, float(obj), float(collb), float(colub))
 
   neginf = typemin(eltype(rowlb))
diff --git a/src/cpx_env.jl b/src/cpx_env.jl
index f8c1f85dc6c..733314fc898 100644
--- a/src/cpx_env.jl
+++ b/src/cpx_env.jl
@@ -1,5 +1,7 @@
 type Env
     ptr::Ptr{Void}
+    num_models::Int
+    finalize_called::Bool
 
     function Env()
       stat = Array(Cint, 1)
@@ -7,7 +9,15 @@ type Env
       if tmp == C_NULL
           error("CPLEX: Error creating environment")
       end
-      new(tmp)
+      env = new(tmp, 0, false)
+      finalizer(env, env -> begin
+                                if env.num_models == 0
+                                    close_CPLEX(env)
+                                else
+                                    env.finalize_called = true
+                                end
+                            end)
+      env
     end
 end
 
@@ -18,6 +28,27 @@ function is_valid(env::Env)
     env.ptr != C_NULL
 end
 
+function notify_new_model(env::Env)
+    env.num_models += 1
+end
+
+function notify_freed_model(env::Env)
+    @assert env.num_models > 0
+    env.num_models -= 1
+    if env.num_models <= 0 && env.finalize_called
+        close_CPLEX(env)
+    end
+end
+
+function close_CPLEX(env::Env)
+    tmp = Ptr{Void}[env.ptr]
+    stat = @cpx_ccall(closeCPLEX, Cint, (Ptr{Void},), tmp)
+    env.ptr = C_NULL
+    if stat != 0
+        throw(CplexError(env, stat))
+    end
+end
+
 function set_logfile(env::Env, filename::String)
   @assert isascii(filename)
   fp = @cpx_ccall(fopen, Ptr{Void}, (Ptr{Cchar}, Ptr{Cchar}), filename, "w")
diff --git a/src/cpx_model.jl b/src/cpx_model.jl
index 04c7c860fec..16ee3f43fea 100644
--- a/src/cpx_model.jl
+++ b/src/cpx_model.jl
@@ -8,13 +8,17 @@ type Model
 end
 
 function Model(env::Env, lp::Ptr{Void})
+    notify_new_model(env)
     model = Model(env, lp, false, false, nothing, Cint[0])
-    finalizer(model, free_problem)
+    finalizer(model, m -> begin
+                              free_problem(m)
+                              notify_freed_model(env)
+                          end)
     set_terminate(model)
     model
 end
 
-function Model(env::Env, name::String)
+function Model(env::Env, name::String="CPLEX.jl")
     @assert is_valid(env)
     stat = Array(Cint, 1)
     tmp = @cpx_ccall(createprob, Ptr{Void}, (Ptr{Void}, Ptr{Cint}, Ptr{Cchar}), env.ptr, stat, name)
@@ -24,24 +28,6 @@ function Model(env::Env, name::String)
     return Model(env, tmp)
 end
 
-# internal function that wraps finalizer for model and environment together
-function _Model(env::Env)
-    @assert is_valid(env)
-    stat = Array(Cint, 1)
-    tmp = @cpx_ccall(createprob, Ptr{Void}, (Ptr{Void}, Ptr{Cint}, Ptr{Cchar}), env.ptr, stat, "CPLEX.jl")
-    if tmp == C_NULL
-        throw(CplexError(model.env, stat))
-    end
-    model = Model(env, tmp, false, false, nothing, Cint[0])
-    finalizer(model, model::Model -> begin
-            tmp = model.env
-            free_problem(model)
-            close_CPLEX(tmp)
-        end)
-    set_terminate(model)
-    return model
-end
-
 function read_model(model::Model, filename::String)
     stat = @cpx_ccall(readcopyprob, Cint, (Ptr{Void}, Ptr{Void}, Ptr{Cchar}, Ptr{Cchar}), model.env.ptr, model.lp, filename, C_NULL)
     if stat != 0
@@ -171,14 +157,6 @@ function free_problem(model::Model)
     end
 end
 
-function close_CPLEX(env::Env)
-    tmp = Ptr{Void}[env.ptr]
-    stat = @cpx_ccall(closeCPLEX, Cint, (Ptr{Void},), tmp)
-    if stat != 0
-        throw(CplexError(env, stat))
-    end
-end
-
 function set_terminate(model::Model)
     stat = @cpx_ccall(setterminate, Cint, (Ptr{Void},Ptr{Cint}), model.env.ptr, model.terminator)
     if stat != 0
diff --git a/test/env.jl b/test/env.jl
new file mode 100644
index 00000000000..22b2ceb894f
--- /dev/null
+++ b/test/env.jl
@@ -0,0 +1,14 @@
+using CPLEX, MathProgBase, Base.Test
+
+@testset "Use loadproblem! twice" begin
+    solver = CplexSolver()
+    # Check that the env for each model is the same
+    m = MathProgBase.LinearQuadraticModel(solver)
+    env = m.inner.env
+    MathProgBase.loadproblem!(m, [1 0], [1, 1], [1, Inf], [1, 0], [0], [Inf], :Min)
+    @test m.inner.env === env
+    @test CPLEX.is_valid(env)
+    MathProgBase.loadproblem!(m, [0 1], [-Inf, 1], [1, 1], [0, 1], [-Inf], [0], :Min)
+    @test m.inner.env === env
+    @test CPLEX.is_valid(env)
+end
diff --git a/test/runtests.jl b/test/runtests.jl
index 23b6b455829..bbc1c8a6552 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -8,6 +8,7 @@ tests = ["low_level_api",
          "qp_01",
          "qp_02",
          "qcqp_01",
+         "env",
          "mathprog"]
 
 for t in tests