Fix a memory leak in program_test.cc
Program::RemoveFixedBocks will happily drop all fixed parameter
and residual blocks. So if it is called on the one and only copy
of a program, like it was being called in the tests, this will
result in a memory leak because we would have lost track of the
fixed parameter and residual blocks.
This is not a problem in actual usage since CreateReducedProgram
will first create a copy of the program and then remove the fixed
blocks from it. But in the tests, we were creating a ProblemImpl
and then calling RemovedFixedBlocks on the underlying program
object causing a memory leak.
The fix to make a copy in the tests and then work on that. I
have also added a warning in program.h
Change-Id: I03a5f7a7f5453aec848451a5c0ace4b065f71e9b
diff --git a/internal/ceres/program.h b/internal/ceres/program.h
index 7f2fc9d..6b9190d 100644
--- a/internal/ceres/program.h
+++ b/internal/ceres/program.h
@@ -131,7 +131,11 @@
// Removes constant parameter blocks and residual blocks with no
// varying parameter blocks while preserving order.
- // TODO(sameeragarwal): Update message here.
+ //
+ // WARNING: It is the caller's responsibility to track ownership of
+ // the parameter and residual blocks removed from the
+ // program. Generally speaking this method should only be called on
+ // a copy of an existing program.
bool RemoveFixedBlocks(vector<double*>* removed_parameter_blocks,
double* fixed_cost,
string* message);
diff --git a/internal/ceres/program_test.cc b/internal/ceres/program_test.cc
index 79adfa6..f273872 100644
--- a/internal/ceres/program_test.cc
+++ b/internal/ceres/program_test.cc
@@ -88,15 +88,15 @@
problem.AddResidualBlock(new BinaryCostFunction(), NULL, &x, &y);
problem.AddResidualBlock(new TernaryCostFunction(), NULL, &x, &y, &z);
- Program* program = problem.mutable_program();
+ Program program(problem.program());
vector<double*> removed_parameter_blocks;
double fixed_cost = 0.0;
string message;
- EXPECT_TRUE(program->RemoveFixedBlocks(&removed_parameter_blocks,
- &fixed_cost,
- &message));
- EXPECT_EQ(program->NumParameterBlocks(), 3);
- EXPECT_EQ(program->NumResidualBlocks(), 3);
+ EXPECT_TRUE(program.RemoveFixedBlocks(&removed_parameter_blocks,
+ &fixed_cost,
+ &message));
+ EXPECT_EQ(program.NumParameterBlocks(), 3);
+ EXPECT_EQ(program.NumResidualBlocks(), 3);
EXPECT_EQ(removed_parameter_blocks.size(), 0);
EXPECT_EQ(fixed_cost, 0.0);
}
@@ -109,15 +109,15 @@
problem.AddResidualBlock(new UnaryCostFunction(), NULL, &x);
problem.SetParameterBlockConstant(&x);
- Program* program = problem.mutable_program();
+ Program program(problem.program());
vector<double*> removed_parameter_blocks;
double fixed_cost = 0.0;
string message;
- EXPECT_TRUE(program->RemoveFixedBlocks(&removed_parameter_blocks,
- &fixed_cost,
- &message));
- EXPECT_EQ(program->NumParameterBlocks(), 0);
- EXPECT_EQ(program->NumResidualBlocks(), 0);
+ EXPECT_TRUE(program.RemoveFixedBlocks(&removed_parameter_blocks,
+ &fixed_cost,
+ &message));
+ EXPECT_EQ(program.NumParameterBlocks(), 0);
+ EXPECT_EQ(program.NumResidualBlocks(), 0);
EXPECT_EQ(removed_parameter_blocks.size(), 1);
EXPECT_EQ(removed_parameter_blocks[0], &x);
EXPECT_EQ(fixed_cost, 9.0);
@@ -134,15 +134,15 @@
problem.AddParameterBlock(&y, 1);
problem.AddParameterBlock(&z, 1);
- Program* program = problem.mutable_program();
+ Program program(problem.program());
vector<double*> removed_parameter_blocks;
double fixed_cost = 0.0;
string message;
- EXPECT_TRUE(program->RemoveFixedBlocks(&removed_parameter_blocks,
- &fixed_cost,
- &message));
- EXPECT_EQ(program->NumParameterBlocks(), 0);
- EXPECT_EQ(program->NumResidualBlocks(), 0);
+ EXPECT_TRUE(program.RemoveFixedBlocks(&removed_parameter_blocks,
+ &fixed_cost,
+ &message));
+ EXPECT_EQ(program.NumParameterBlocks(), 0);
+ EXPECT_EQ(program.NumResidualBlocks(), 0);
EXPECT_EQ(removed_parameter_blocks.size(), 3);
EXPECT_EQ(fixed_cost, 0.0);
}
@@ -161,15 +161,15 @@
problem.AddResidualBlock(new BinaryCostFunction(), NULL, &x, &y);
problem.SetParameterBlockConstant(&x);
- Program* program = problem.mutable_program();
+ Program program(problem.program());
vector<double*> removed_parameter_blocks;
double fixed_cost = 0.0;
string message;
- EXPECT_TRUE(program->RemoveFixedBlocks(&removed_parameter_blocks,
- &fixed_cost,
- &message));
- EXPECT_EQ(program->NumParameterBlocks(), 1);
- EXPECT_EQ(program->NumResidualBlocks(), 1);
+ EXPECT_TRUE(program.RemoveFixedBlocks(&removed_parameter_blocks,
+ &fixed_cost,
+ &message));
+ EXPECT_EQ(program.NumParameterBlocks(), 1);
+ EXPECT_EQ(program.NumResidualBlocks(), 1);
}
TEST(Program, RemoveFixedBlocksNumEliminateBlocks) {
@@ -186,15 +186,15 @@
problem.AddResidualBlock(new BinaryCostFunction(), NULL, &x, &y);
problem.SetParameterBlockConstant(&x);
- Program* program = problem.mutable_program();
+ Program program(problem.program());
vector<double*> removed_parameter_blocks;
double fixed_cost = 0.0;
string message;
- EXPECT_TRUE(program->RemoveFixedBlocks(&removed_parameter_blocks,
- &fixed_cost,
- &message));
- EXPECT_EQ(program->NumParameterBlocks(), 2);
- EXPECT_EQ(program->NumResidualBlocks(), 2);
+ EXPECT_TRUE(program.RemoveFixedBlocks(&removed_parameter_blocks,
+ &fixed_cost,
+ &message));
+ EXPECT_EQ(program.NumParameterBlocks(), 2);
+ EXPECT_EQ(program.NumResidualBlocks(), 2);
}
TEST(Program, RemoveFixedBlocksFixedCost) {
@@ -211,10 +211,10 @@
problem.AddResidualBlock(new BinaryCostFunction(), NULL, &x, &y);
problem.SetParameterBlockConstant(&x);
- Program* program = problem.mutable_program();
+ Program program(problem.program());
double expected_fixed_cost;
- ResidualBlock *expected_removed_block = program->residual_blocks()[0];
+ ResidualBlock *expected_removed_block = program.residual_blocks()[0];
scoped_array<double> scratch(
new double[expected_removed_block->NumScratchDoublesForEvaluate()]);
expected_removed_block->Evaluate(true,
@@ -226,12 +226,12 @@
vector<double*> removed_parameter_blocks;
double fixed_cost = 0.0;
string message;
- EXPECT_TRUE(program->RemoveFixedBlocks(&removed_parameter_blocks,
- &fixed_cost,
- &message));
+ EXPECT_TRUE(program.RemoveFixedBlocks(&removed_parameter_blocks,
+ &fixed_cost,
+ &message));
- EXPECT_EQ(program->NumParameterBlocks(), 2);
- EXPECT_EQ(program->NumResidualBlocks(), 2);
+ EXPECT_EQ(program.NumParameterBlocks(), 2);
+ EXPECT_EQ(program.NumResidualBlocks(), 2);
EXPECT_DOUBLE_EQ(fixed_cost, expected_fixed_cost);
}
