Tpetra::CrsMatrix: Add internal thread-safe version of getCrsGraph
Created by: mhoemmen
@trilinos/tpetra Blocks: #941 (closed)
CrsMatrix::getCrsGraph
returns a Teuchos::RCP
of
the matrix's graph. This is not currently thread safe, because RCP's
reference count is not currently thread safe. In particular, RCP's
copy constructor, assignment operator (operator=
), and destructor all
modify the reference count, so none of these are thread safe. The
getCrsGraph
method must increment the graph's reference count, since
it returns the RCP by value. Thus, it cannot be thread safe.
Returning const RCP&
is not safe (threads or no), because callers
can access it even after the lifetime of the graph has ended.
Today, I was reminded of this lack of thread safety the hard way,
while host-thread-parallelizing CrsMatrix::mergeRedundantValues
(see
#941 (closed)). Many methods in CrsMatrix look "harmless" -- they appear just
to access some local state like the number of rows owned by the
calling process -- but are not thread safe. This is because they call
getCrsGraph
, then ask the graph for information. Many of the
methods that ask the graph for information are themselves thread safe,
but the getCrsGraph
call itself is the problem. I was able to fix
the thread safety issue in my changes to mergeRedundantValues
, by
lifting the relevant call (isStorageOptimized()
) outside of the
host-thread-parallel loop. However, I would like to forestall this
issue for future work.
I propose the following fix: add a new private method, getCrsGraphRef
, to CrsMatrix.
The new method will return const crs_graph_type&
. Its implementation must
carefully avoid any RCP methods that would change the RCP's reference
count.