Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-19794

QCache operator[] is marked const but modifies internal structures and is therefore only usable by multi-threaded code using a write-lock or mutex.

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Open
    • Priority: P2: Important
    • Resolution: Unresolved
    • Affects Version/s: 4.6.3, 5.1.0 Beta 1
    • Fix Version/s: None
    • Labels:
      None

      Description

      QCache::operator[] is marked const. It is reasonable for the developer to assume that it is usable via multi-threaded code with a read lock, because it is const. However, it delegates to object(), which is const but uses const_cast to unconst 'this' and call relink. Relink is definitely not const, since it relinks a found node so that it is first in the linked list. It is therefore possible to multiple threads to end up in relink and for memory to become corrupted.

      I would guess that the developer implementing object() chose to reuse relink rather than implement its (relatively simple) logic, so that successive accesses to an object retrieved using operator[] or object() are more efficient.

      I'd suggest, though, that constness is important and should not be sacrificed in the interests of possible optimization, and that object need only be:

      inline T *object(const Key &key) const {
      typename QHash<Key, Node>::const_iterator i = hash.find(key);
      if (i == hash.constEnd())

      { return 0; }

      return *i;
      }

      At a minimum, the documentation should make it clear that operator[] and object() are not const.

        Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

            Assignee:
            Unassigned Unassigned
            Reporter:
            benlast Ben Last
            Votes:
            4 Vote for this issue
            Watchers:
            9 Start watching this issue

              Dates

              Created:
              Updated:

                Gerrit Reviews

                There are no open Gerrit changes