New issue
Advanced search Search tips

Issue 835611 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocking:
issue 612287



Sign in to add a comment

Direct-leak in content::CacheStorageCache::MatchDidMatchAll

Project Member Reported by ClusterFuzz, Apr 22 2018

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=5612108163842048

Fuzzer: inferno_twister_c
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: Direct-leak
Crash Address: 
Crash State:
  content::CacheStorageCache::MatchDidMatchAll
  void base::internal::FunctorTraits<void
  MakeItSo<void
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=544931:544932

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5612108163842048

Additional requirements: Requires HTTP

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by ClusterFuzz, Apr 22 2018

Components: Blink>Storage>CacheStorage Internals>Core
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 2 by ClusterFuzz, Apr 22 2018

Labels: Test-Predator-Auto-Owner
Owner: lucmult@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/626c99e870b04e474408a5b35e97f260b9d787ba (Convert Cache Storage IPC to Mojo.).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
I couldn't reproduce the issue locally:
$ prodaccess && /google/data/ro/teams/clusterfuzz-tools/releases/clusterfuzz reproduce 5612108163842048 --current   
...
Original crash type: Direct-leak
Original crash state:
  NULL
...
The stacktrace doesn't match the original stacktrace.
Try again (3 times). Press Ctrl+C to stop trying to reproduce.


This makes fixing this issue more complicated.
Cc: mek@chromium.org jsb...@chromium.org cmumford@chromium.org
Status: Started (was: Assigned)
I think I found something, but I can't fully confirm since I can't reproduce the failure locally.

https://chromium-review.googlesource.com/c/chromium/src/+/1023731
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 25 2018

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

commit 61d14eb7c388d5a416672a6382d5311b323976fc
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Wed Apr 25 06:55:24 2018

Don't release the unique_ptr so it clears memory

ServiceWorkerResponse type doesn't provide move constructor nor
assignment, thus when moving to vector it creates a copy, so we need
unique_ptr to delete the memory used by its value, otherwise it leaks.

Bug:  835611 
Change-Id: I1276b5e3935f9759b8d9dd84df3a958b1dd9d25e
Reviewed-on: https://chromium-review.googlesource.com/1023731
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Luciano Pacheco (SYD) <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553473}
[modify] https://crrev.com/61d14eb7c388d5a416672a6382d5311b323976fc/content/browser/cache_storage/cache_storage_dispatcher_host.cc

Project Member

Comment 6 by ClusterFuzz, Apr 25 2018

ClusterFuzz has detected this issue as fixed in range 553468:553474.

Detailed report: https://clusterfuzz.com/testcase?key=5612108163842048

Fuzzer: inferno_twister_c
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: Direct-leak
Crash Address: 
Crash State:
  content::CacheStorageCache::MatchDidMatchAll
  void base::internal::FunctorTraits<void
  MakeItSo<void
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=544931:544932
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=553468:553474

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5612108163842048

Additional requirements: Requires HTTP

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 7 by ClusterFuzz, Apr 25 2018

Labels: ClusterFuzz-Verified
Status: Verified (was: Started)
ClusterFuzz testcase 5612108163842048 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Blocking: 612287
Project Member

Comment 9 by bugdroid1@chromium.org, May 1 2018

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

commit 9aed4f3692399b4ad59ae472367105db72e83896
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Tue May 01 01:39:25 2018

Remove unnecessary parameter CacheStorageCacheHandle

A few methods on CacheImpl had this parameter even though they don't
use it, also this class has this same object as a member, so it isn't
needed as parameter at all.

Bug:  835611 
Change-Id: I81c5694c7bbbbfd19b559265114c83e508649e77
Reviewed-on: https://chromium-review.googlesource.com/1023675
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Commit-Queue: Luciano Pacheco (SYD) <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554961}
[modify] https://crrev.com/9aed4f3692399b4ad59ae472367105db72e83896/content/browser/cache_storage/cache_storage_dispatcher_host.cc

Sign in to add a comment