Uploaded image for project: 'WiredTiger'
  1. WiredTiger
  2. WT-4909

Fix potential code caching/re-ordering issue in __search_insert_append

    • Type: Icon: Technical Debt Technical Debt
    • Resolution: Fixed
    • Priority: Icon: Minor - P4 Minor - P4
    • 4.2.1, WT3.2.1, 4.3.1
    • Affects Version/s: None
    • Component/s: None
    • Labels:
      None
    • 2
    • Storage Engines 2019-07-29

      My reading of this code in __search_insert_append():

      if ((ins = WT_SKIP_LAST(ins_head)) == NULL)
              return (0);
      key.data = WT_INSERT_KEY(ins);
      key.size = WT_INSERT_KEY_SIZE(ins);
      
      WT_RET(__wt_compare(session, collator, srch_key, &key, &cmp));
      if (cmp >= 0) {
              /*
               * !!!
               * We may race with another appending thread.
               *
               * To catch that case, rely on the atomic pointer read above
               * and set the next stack to NULL here.  If we have raced with
               * another thread, one of the next pointers will not be NULL by
               * the time they are checked against the next stack inside the
               * serialized insert function.
               */
              for (i = WT_SKIP_MAXDEPTH - 1; i >= 0; i--) {
                      cbt->ins_stack[i] = (i == 0) ? &ins->next[0] :
                          (ins_head->tail[i] != NULL) ?
                          &ins_head->tail[i]->next[i] : &ins_head->head[i];
                      cbt->next_stack[i] = NULL;
              }
              cbt->compare = -cmp;
              cbt->ins = ins;
              cbt->ins_head = ins_head;
              *donep = 1;
      }
      

      worries me. I think we require ins be assigned before it's used in the if clause, and while it's vanishingly unlikely that wouldn't happen, there's nothing that stops the compiler from repeating the read inside the if clause, and I think that would cause a problem.

      cc michael.cahill, haribabu.kommi

            Assignee:
            alex.cameron@mongodb.com Alex Cameron (Inactive)
            Reporter:
            keith.bostic@mongodb.com Keith Bostic (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: