From e2482a0855e2e5b0d3a1a0ea3b64b7bb05e75dc6 Mon Sep 17 00:00:00 2001 From: David Wells Date: Sat, 11 Feb 2017 17:29:44 -0500 Subject: [PATCH] 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. --- include/deal.II/lac/petsc_parallel_vector.h | 5 ++++- source/lac/petsc_parallel_vector.cc | 14 +++++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/include/deal.II/lac/petsc_parallel_vector.h b/include/deal.II/lac/petsc_parallel_vector.h index 2713b63072..e61ae3d9c7 100644 --- 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 6073e3dc55..bd519019c8 100644 --- 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; } -- 2.39.5