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); }