https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118854
--- Comment #8 from Nicholas Williams <nicholas at nicholaswilliams dot net> ---
(In reply to Andrew Pinski from comment #4)
> fsanitize=address just changes timing slightly to give a race condition
> between the thread and the constructor to show up.
That's what I had assumed.
(In reply to Andrew Pinski from comment #5)
> One way of fixing this is to move _pInstance to be a static variable
> inside instance function which makes the initialization dynamic and
> correctly ordered.
This is not correct. I still get a heap-use-after free in the copy constructor
with this code:
instance()
{
static std::shared_ptr< Resource > pInstance = std::make_shared< Resource
>();
std::shared_ptr< Resource > ptr( pInstance );
if ( !ptr )
{
throw std::runtime_error( "Resource has already destructed" );
}
return ptr;
}
(In reply to Andrew Pinski from comment #3)
> The bug is in your code.
>
> > If I reverse the order of these two statements
>
> That is correct. You have an order of initialization issue.
I disagree with the contention that the bug is in my code. I do agree that
there's a disparity in initialization order, but it's considerably more
difficult to properly control initialization order when you have a
multi-hundred-CPP-file application.
Notably, the copy constructor is declared noexcept. While I know that's
syntatically about *exceptions*, the *implication* is that it should be safe to
call that construct and not experience errors. A segfault/heap-after-use is an
error.
More importantly, though, the C++ documentation for shared_ptr mentions thread
safety in a few ways:
> To satisfy thread safety requirements, the reference counters are typically
> incremented using an equivalent of std::atomic::fetch_add with
> std::memory_order_relaxed.
And:
> If multiple threads of execution access the same shared_ptr object without
> synchronization and any of those accesses uses a non-const member function
> of shared_ptr then a data race will occur
But the copy constructor argument is const, which means only const member
functions / data members of that argument can be used, so it should be thread
safe.
There is a strong implication throughout the totality of the documentation/spec
for shared_ptr and its copy constructor that copying a shared_ptr in a
multi-threaded environment should be safe.
