New issue
Advanced search Search tips

Issue 709848 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 594639



Sign in to add a comment

HeapHashTable fails to call elements' destructors

Project Member Reported by yhirano@chromium.org, Apr 10 2017

Issue description

See the following code in HeapHashTable::ExpandBuffer.

  ValueType* original_table = table_;

  ValueType* temporary_table = AllocateTable(old_table_size);
  for (unsigned i = 0; i < old_table_size; i++) {
    if (&table_[i] == entry)
      new_entry = &temporary_table[i];
    if (IsEmptyOrDeletedBucket(table_[i])) {
      ...
    } else {
      Mover<ValueType, Allocator,
            Traits::template NeedsToForbidGCOnMove<>::value>::
          Move(std::move(table_[i]), temporary_table[i]);
    }
  }
  table_ = temporary_table;

  if (Traits::kEmptyValueIsZero) {
    memset(original_table, 0, new_table_size * sizeof(ValueType));
  } else {
    for (unsigned i = 0; i < new_table_size; i++)
      InitializeBucket(original_table[i]);
  }

The function clears |original_table_| without calling destructors of each valid element.
 

Comment 1 by kouhei@chromium.org, Apr 10 2017

Blocking: 594639
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f3e971b60293bb5f7e6bd8de03b7f3ea77a2be13

commit f3e971b60293bb5f7e6bd8de03b7f3ea77a2be13
Author: yhirano <yhirano@chromium.org>
Date: Mon Apr 10 05:43:50 2017

Call elements' destructors in HashTable

HashTable::ExpandBuffer doesn't call elements' destructors appropriately. It
relys on a wrong assumption that it doesn't have to call the destructor on
a moved out object.

This CL fixes that.

BUG= 709848 

Review-Url: https://codereview.chromium.org/2807053002
Cr-Commit-Position: refs/heads/master@{#463174}

[modify] https://crrev.com/f3e971b60293bb5f7e6bd8de03b7f3ea77a2be13/third_party/WebKit/Source/platform/heap/HeapTest.cpp
[modify] https://crrev.com/f3e971b60293bb5f7e6bd8de03b7f3ea77a2be13/third_party/WebKit/Source/platform/wtf/HashTable.h

Status: Fixed (was: Assigned)

Sign in to add a comment