This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 438 - specialization of class SparseLU for UmfPack violates rule of three
Summary: specialization of class SparseLU for UmfPack violates rule of three
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Unsupported modules (show other bugs)
Version: 3.0
Hardware: All All
: Normal major
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 3.1
  Show dependency treegraph
 
Reported: 2012-03-19 12:08 UTC by Maxim Ivanov
Modified: 2019-12-04 11:33 UTC (History)
1 user (show)



Attachments

Description Maxim Ivanov 2012-03-19 12:08:17 UTC
In the source file unsupported/Eigen/src/SparseExtra/UmfPackSupport.h of Eigen version 3.0.1 one can find the specialization of class template SparseLU for UmfPack, which defines the following destructor:

    ~SparseLU()
    {
      if (m_numeric)
        umfpack_free_numeric(&m_numeric,Scalar());
    }

where m_numeric is an opaque member pointer to Umfpack library data. This definition tries to provide some resource management of the data, however the class doesn't define a copy constructor nor the copy assignment operator, as the C++ rule of three suggests. This leaves the class with implicitly generated copy constructor and copy assignment operator which obviously don't handle the pointer in correct way.

This easily leads to incorrect behavior of the class in various situations, for example:

class Test {
    Eigen::SparseMatrix<double> equations;
    Eigen::SparseLU<decltype(equations), Eigen::UmfPack> equations_lu;
    // ...
public:
    Test(...);
};

Test::Test(...) {
    // populate the default-constructed equations member
    // ...

    // perform the decomposition
    equations_lu = {equations};

    // ...
}


In this (and notably many others) usage scenario equations_lu will have incorrect m_numeric value after the initialization.

My workaround to this bug was using the SparseLU::compute method to reinitialize equations_lu in-place.

I'd suggest using a smart pointer with custom allocator to manage the lifetime of m_numeric.
Comment 1 Gael Guennebaud 2012-03-26 18:38:31 UTC
yes, you are right. Though this SparseLU<> class and the like are deprecated, we should check their replacements for the soon coming 3.1. In particular, in this case we should forbidden copy ctor and assignment.
Comment 2 Gael Guennebaud 2012-06-06 08:24:05 UTC
This has been recently fixed.
Comment 3 Nobody 2019-12-04 11:33:27 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/438.

Note You need to log in before you can comment on or make changes to this bug.