Move problem with mesh

[vhirtham]

Recently we (pmueller2 and me) encountered some problems using the MeshFem. If you move your mesh, there is a good chance to get an access violation, depending on your compilers mood. It seems like it is related to a boost pointer container, that is not movable or at least, does not support move semantics as pmueller2 found out.

3 Steps to access violation:

1. This works:

   
   MeshFem mesh = UnitMeshFem::CreateLines(numElements); 

DofType displ("displacements", 1); const auto& interpolation = mesh.CreateInterpolation(InterpolationTrussLinear(1)); AddDofInterpolation(&mesh, displ, interpolation);

2. Still working:
~~~cpp
MeshFem meshUnit = UnitMeshFem::CreateLines(numElements);
MeshFem mesh = UnitMeshFem::Transform(std::move(meshUnit), 
                              [](Eigen::VectorXd vec) { return 1.0 * vec; }); 

... // same as above

3. Welcome to access violation wonderland:

   
   MeshFem mesh = UnitMeshFem::Transform(UnitMeshFem::CreateLines(numElements),
                                 [](Eigen::VectorXd vec) { return SpecimenLength * vec; });

... // same as above

The access violation occurs during the call of `AddDofInterpolation`. Only thing that changed from step 2. to 3. is that I removed the temporary mesh by using the result of `CreateLines` directly in the function call. As I said, it seems to be related to this member of MeshFem:

~~~cpp
boost::ptr_vector<InterpolationSimple> mInterpolations;

How do we want to handle this problem? Just replace the container by a std::vector<std::unique_ptr<InterpolationSimple>>? Write a custom move ctor / assignment operator? Other solution?

[TTitscher]

NIce find! Note that a method that returns a MeshFem by value not actually performs a move (copy elision). This may explain the different cases you described. The error only occurs if the object is actually moved and the object that is moved from really goes out of scope.

Anyways. A similar issue was solved here c92f07888bcbb. In the future, I aim for copyable elements. For now, your proposed solution with vector<unique_ptr> seems reasonable and fast. I would be great if you could:

• push a first commit with a failing test that documents this issue and travis fails
• a second commit that fixes the test/bug

Would you like to fix that yourselves?

[vhirtham]

Can do it tomorrow, if you can wait until then :stuck_out_tongue:

Note that a method that returns a MeshFem by value not actually performs a move (copy elision). This may explain the different cases you described.

Yeah, that is what I ment with "depending on your compilers mood" :smile:

[vhirtham]

I used the current history data branch to add a test and the fix. The test is rather short:

BOOST_AUTO_TEST_CASE(MeshMovabilityError)
{
    NuTo::MeshFem mesh = NuTo::UnitMeshFem::CreateLines(1);
    {
        NuTo::MeshFem tempMesh = NuTo::UnitMeshFem::CreateLines(1);
        mesh = std::move(tempMesh);
    }
    auto& coordinateElement = mesh.Elements[0].CoordinateElement();
    coordinateElement.GetDofDimension();
}

We create a mesh. Then we open a new scope and create a temporary mesh. Afterwards we move the temporary into the first mesh. Finally the scope is closed and the temporary mesh is destroyed.

We now want to get the dof dimension of a coordinate node which needs an interpolation. However, boost::ptr_vector does not support move semantics and something goes wrong when the temporary mesh goes out of scope and destroys his interpolation. Somehow our original mesh still points to some freed memory of the temporary mesh. This causes the observed error. You can check this by removing the additional scope, so that the temporary mesh is not destroyed. In this case the test won't fail anymore.

[TTitscher]

I used the current history data branch to add a test and the fix.

It would have been nice to have a separate branch for that bug. But as soon as we merge PR #144 this will be done. Nice.