Fix, and deprecate, a PETSc parallel vector copy constructor.

The former implementation of this copy constructor contained a subtle bug: since
VectorBase::operator= is implicitly defined, calling it simply copies (byte for
byte) the members of a VectorBase. This is erroneous because now two different
destructors will call VecDestroy on the underlying Vec.

The fix is simple: (nearly) preserve the current behavior by just deep copying
the vector.

Since one should never have an explicit instance of VectorBase (and there is no
reasonable way to copy a parallel vector and then enforce a possibly different
local_size) this constructor I also marked this constructor for deprecation: one
should use one of the concrete classes (Vector or MPI::Vector) in applications.

diff --git a/include/deal.II/lac/petsc_parallel_vector.h b/include/deal.II/lac/petsc_parallel_vector.h
index 2713b630728b79f254c60ac2038b7448007d81ba..e61ae3d9c737973f4afd7dc1d6de697f021d934d 100644 (file)
--- a/include/deal.II/lac/petsc_parallel_vector.h
+++ b/include/deal.II/lac/petsc_parallel_vector.h
@@ -227,10 +227,13 @@ namespace PETScWrappers
        *
        * @arg communicator denotes the MPI communicator over which the
        * different parts of the vector shall communicate
+       *
+       * @deprecated The use of objects that are explicitly of type VectorBase
+       * is deprecated: use PETScWrappers::MPI::Vector instead.
        */
       explicit Vector (const MPI_Comm     &communicator,
                        const VectorBase   &v,
-                       const size_type     local_size);
+                       const size_type     local_size) DEAL_II_DEPRECATED;
 
       /**
        * Construct a new parallel ghosted PETSc vector from an IndexSet.

diff --git a/source/lac/petsc_parallel_vector.cc b/source/lac/petsc_parallel_vector.cc
index 6073e3dc556756ba1b5553130d7adfe46643cb3d..bd519019c8c367c66d9fe7436a0e7448f0361acb 100644 (file)
--- a/source/lac/petsc_parallel_vector.cc
+++ b/source/lac/petsc_parallel_vector.cc
@@ -56,11 +56,19 @@ namespace PETScWrappers
                     const VectorBase  &v,
                     const size_type   local_size)
       :
+      VectorBase (v),
       communicator (communicator)
     {
-      Vector::create_vector (v.size(), local_size);
-
-      VectorBase::operator = (v);
+      // In the past (before it was deprecated) this constructor did a
+      // byte-for-byte copy of v. This choice resulted in two problems:
+      // 1. The created vector will have the same size as v, not local size.
+      // 2. Since both the created vector and v maintain ownership of the same
+      // PETSc Vec, both will try to destroy it: this does not make sense.
+      //
+      // For the sake of backwards compatibility, preserve the behavior of the
+      // copy, but correct the ownership bug. Note that in both this (and the
+      // original) implementation local_size is ultimately unused.
+      (void)local_size;
     }