[Qt-interest] QHash::const_iterator not reentrant?

[K. Frank]

Hello Olivier!

On Thu, Aug 18, 2011 at 12:56 PM, BOUCARD Olivier
<boucard_olivier at yahoo.fr> wrote:
> Hi everybody,
> I have a problem with QHash and const_iterator.

I do not see the cause of the problem.  (I have not tried to replicate with
test code.)

> I have two QHash instance with their respective const_iterator in two
> different thread.

I would think, offhand, that this should be fine, provided that the
two instances
of QHash really are different (i.e., not aliased, somehow).  One would hope
that QHash is designed so that different instances of QHash don't interfere
with one another.

Are you sure that your two instances really are different?

> And it cause crash when I iterate on both at the same time.
> I have made a little research and found that there is a static function
> QHashData::Node *QHashData::nextNode(Node *node).

I'm looking at the QHashData::nextNode code, and I don't see any obvious
problem.  It looks like all of the function's internal variables are local, and
therefore on the thread-specific stack, so things should be okay.

> And I think the problem come from here, the code seems not reentrant:

(A note on terminology:  I would call a function reentrant if it can call itself
recursively, perhaps indirectly.  A thread-safe function can be called
simultaneously by more than one thread.  Although most things that
break reentrancy break thread safety, the two concepts are not identical.
It is possible for a reentrant function not to be thread safe, and vice versa.
I think you mean thread safe in the context of your post.)

> QHashData::Node *QHashData::nextNode(Node *node)
> {
>     union {
>         Node *next;
>         Node *e;
>         QHashData *d;
>     };
>     next = node->next;
>     Q_ASSERT_X(next, "QHash", "Iterating beyond end()");
>     if (next->next)
>         return next;
>
>     int start = (node->h % d->numBuckets) + 1;
>     Node **bucket = d->buckets + start;
>     int n = d->numBuckets - start;
>     while (n--) {
>         if (*bucket != e)
>             return *bucket;
>         ++bucket;
>     }
>     return e;
> }

I just don't see anything fishy here in the QHashData::nextNode code.
It looks to me to be thread safe.  (It's also hypothetically reentrant, but
there is no way for it to call itself as it calls no functions -- unless there
is some unexpected operator overloading going on that makes some
function call not obvious.)

> Someone could confirm that, please.
> Any alternative, workaround, ... if it is confirmed?

Well, if there really is some failure of thread safety that I've overlooked, you
could always synchronize your calls to QHashData::nextNode (or the use
of the QHash::const_iterator that presumably call QHashData::nextNode)
with a mutex.  Doing so would, in principle, cause a performance hit, but
whether this would matter in practice would depend on the details of what
you are doing.

>
> Thank you.
> Olivier.

Good luck.

K. Frank