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

refcnt on error path in __wt_lsm_tree_truncate

    XMLWordPrintable

    Details

    • Type: Task
    • Status: Closed
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: WT2.3.0
    • Component/s: None
    • Labels:

      Description

      @agorrod, clang says the initial assignment to locked in __wt_lsm_tree_truncate:

              WT_UNUSED(cfg);
              chunk = NULL;
              locked = 0;
       
              /* Get the LSM tree. */
              WT_RET(__wt_lsm_tree_get(session, name, 1, &lsm_tree));
      

      is dead, and technically, it's correct because all of the paths that lead to a read of that variable are after a subsequent assignment to it.

      I'd ignore that complaint, myself, but while I was staring at the code, I noticed we're not decrementing the tree's refcnt on error (the error path doesn't call __wt_lsm_tree_release). Is that correct? If not, maybe this change would be appropriate?

      diff --git a/src/lsm/lsm_tree.c b/src/lsm/lsm_tree.c
      index 6d12ff6..3f3a519 100644
      --- a/src/lsm/lsm_tree.c
      +++ b/src/lsm/lsm_tree.c
      @@ -931,7 +931,6 @@ __wt_lsm_tree_truncate(
       
              WT_UNUSED(cfg);
              chunk = NULL;
      -       locked = 0;
       
              /* Get the LSM tree. */
              WT_RET(__wt_lsm_tree_get(session, name, 1, &lsm_tree));
      @@ -941,7 +940,6 @@ __wt_lsm_tree_truncate(
       
              /* Prevent any new opens. */
              WT_RET(__wt_lsm_tree_lock(session, lsm_tree, 1));
      -       locked = 1;
       
              /* Create the new chunk. */
              WT_ERR(__wt_calloc_def(session, 1, &chunk));
      @@ -955,12 +953,10 @@ __wt_lsm_tree_truncate(
              WT_ERR(__wt_lsm_meta_write(session, lsm_tree));
       
              WT_ERR(__lsm_tree_start_worker(session, lsm_tree));
      -       locked = 0;
      -       WT_ERR(__wt_lsm_tree_unlock(session, lsm_tree));
      +
      +err:   WT_TRET(__wt_lsm_tree_unlock(session, lsm_tree));
              __wt_lsm_tree_release(session, lsm_tree);
       
      -err:   if (locked)
      -               WT_TRET(__wt_lsm_tree_unlock(session, lsm_tree));
              if (ret != 0) {
                      if (chunk != NULL) {
                              (void)__wt_schema_drop(session, chunk->uri, NULL);
      

      would be a reasonable change.

        Attachments

          Activity

            People

            • Assignee:
              alexander.gorrod Alexander Gorrod
              Reporter:
              keith.bostic Keith Bostic
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: