QSharedPointer base class (ExternalRefCount) has thread unsafe constructors

In file ./src/corelib/tools/qsharedpointer_impl.h

I believe the copy constructor and one more constructor of the base class ExternalRefCount of QSharedPointer (and hence the derived class QSharedPointer too)
are incorrectly implemented (causing them thread unsafe) unlike the assignment operator of QSharedPointer which is thread safe.

Constructors allow wrongly increasing the strong reference count on the source object , even with current reference count zero of the source object unlike the ExternalRefCount::internalCopy used for assigment operator QSharedPointer.
The implementations of constructor and assignment operator should not treat parameters to functions (source of copying) "const ExternalRefCount<T> &other" differently.
They might treat differently the target of the copying (this object) as at the construction time of this object is definitely no former object to destroy/delete.
Current situation is they do different things on source object.
Assuming
T
QSharedPointer<T> source(new T);
then

QSharedPointer<T> target(source);

should do exactly the same/or equivalent in all situations as

QSharedPointer<T> target;
target=source;

And for some corner cases (relevant in multithreading context) they are not equivalent. Reference counting should be thread safe even in corner cases.

Assigment operators of QSharedPointer calls
void ExternalRefCount::internalCopy(const ExternalRefCount<X> &other) which in turn calls
void ExternalRefCount internalSet(Data *o, T *actual); (see below)

This last function handles correctly (in multithreading) the situation like d && (!d->strongref )
and more importantly correctly prevents to call other.d->ref() when other.d->strongref==0 , it prevents increasing strong reference counter from zero, (because object could have been destroyed and there is no valid source object to copy)
If d is nonzero, it does not mean the d->strongref (the one relevant for controlled object) is nonzero too in multi-thraeding context.
Simply "if (d) ref(); " is not sufficient.

There is no such protection in copy constructor
inline ExternalRefCount(const ExternalRefCount<T> &other) : Basic<T>(other), d(other.d)

{ if (d) ref(); }
and arbitrary templated constructor, which are very similar.
template <class X>
inline ExternalRefCount(const ExternalRefCount<X> &other) : Basic<T>(other.value), d(other.d)
{ if (d) ref(); }

which means there are cases where assignment operator on QSharedPointer would work but copy constructor would not.

I suggest the copy constructor and the other constructor are changed to call internalCopy like this

inline ExternalRefCount(const ExternalRefCount<T> &other) : Basic<T>(0), d(0)

{ internalCopy(other); }

template <class X>
inline ExternalRefCount(const ExternalRefCount<X> &other) : Basic<T>(0), d(0)

{ internalCopy(other); }

The bugs relating to thread unsafe operation are very hard to track and even reproduce.
So I cannot yet provide the code which reliably demonstrates the problem in multi-threaded environment.

I believe this explanation is sufficient.

Thank you
Bye

__Relevant functions_____________________________________________________________

template <class X>
inline void internalCopy(const ExternalRefCount<X> &other)

{ internalSet(other.d, other.data()); }

inline void internalSet(Data *o, T *actual)
{
if (o) {
// increase the strongref, but never up from zero
// or less (-1 is used by QWeakPointer on untracked QObject)
register int tmp = o->strongref;
while (tmp > 0)

{ // try to increment from "tmp" to "tmp + 1" if (o->strongref.testAndSetRelaxed(tmp, tmp + 1)) break; // succeeded tmp = o->strongref; // failed, try again }

if (tmp > 0)
o->weakref.ref();
else
o = 0;
}
if (d && !deref())
delete d;
d = o;
this->value = d && d->strongref ? actual : 0;
}