New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 725671 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

Temporary directories are not being cleaned up

Reported by avin...@aczoom.com, May 23 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36

Steps to reproduce the problem:
1. Run chrome. Leave running for hours, maybe days?
2. Look at %TEMP% directory
3. See tens, maybe hundreds of directories named: number_number, ex:  8772_4235, 8772_27945, etc

What is the expected behavior?
Chrome should not create so many junk directories. It should clean up after itself, before creating a new directory, delete old one.

Even better, since this bug keeps coming back, chrome should create a top-level directory in %TEMP% we can at least remove all the Chrome junk in one command instead of hunting down 100s of these directories.

What went wrong?
Junk left around, 100s of directories.

Each with three files:

05/19/2017  07:26 AM             1,424 crl-set
05/19/2017  07:26 AM                66 manifest.fingerprint
05/19/2017  07:26 AM                34 manifest.json

Did this work before? Yes 

Chrome version: 58.0.3029.110  Channel: stable
OS Version: 10.0
Flash Version: 

See  bug 33122  also, but looks like that has been archived, so creating this new bug.
 
Cc: pbomm...@chromium.org ligim...@chromium.org
Components: Internals>Installer
Labels: Needs-Triage-M58
Prudhvi, can you please triage this issue.
Cc: waff...@chromium.org
cc'ing Joshua.
Are you running any anti-virus software (or other software that might be opening these files as they placed on disk)?

Comment 4 by avin...@aczoom.com, May 24 2017

I think Windows 10 has its own virus checker, I'm not running any anti-virus other  than standard windows settings.

I just ran procexp Process Explorer, to see which process has opened which files. But not sure I can find the  info I need there.
I do see
C:\Users\...\Temp\etilqs_p0dhokcJnagW5rb
and more opened by chrome.exe

All this just started happening a week or two ago, probably after the  recent Chrome update.

I manually check Temp dir often, so this was certainly not happening a few months ago. I also restarted Chrome after manually cleaning out these directories, but they have now reappeared.

Comment 5 by sorin@chromium.org, May 24 2017

Owner: sorin@chromium.org
Status: Started (was: Unconfirmed)

Comment 6 by sorin@chromium.org, May 24 2017

Labels: -Pri-2 Pri-1

Comment 7 by sorin@chromium.org, May 25 2017

Components: -Internals>Installer Internals>Installer>Components
Labels: ReleaseBlock-Stable M-60 M-59
Based on offline chat with sorin@ this need a client side change as well hence marking M59 RBS.
Cc: abdulsyed@chromium.org
Project Member

Comment 10 by bugdroid1@chromium.org, May 25 2017

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

commit 3ef0c4a62c3bbc2cea73b2821fa6637570c94888
Author: sorin <sorin@chromium.org>
Date: Thu May 25 17:58:14 2017

Clean up the unpack directory for CRLSet component.

In the current implementation of the component updater, the component
installer owns the unpack path of a component and it is responsible
for deleting the unpack path before the execution flow returns to
the component updater during the update of a component.

For most components, this clean up is done in the implementation of
the DefaultComponentInstaller class. However, CRLSet is one of the few
components which is not reusing the DefaultComponentInstaller for
its component installer. As a result, it is leaking the content of
the unpack path after each update cycle.

It is not clear when this regression has been introduced.

BUG= 725671 

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

[modify] https://crrev.com/3ef0c4a62c3bbc2cea73b2821fa6637570c94888/chrome/browser/net/crl_set_fetcher.cc

Comment 11 by sorin@chromium.org, May 25 2017

Status: Fixed (was: Started)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-59; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-59 label, otherwise remove Merge-TBD label. Thanks.

Comment 13 by sorin@chromium.org, May 25 2017

Labels: -Merge-TBD Merge-Request-59
Project Member

Comment 14 by sheriffbot@chromium.org, May 26 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 15 by bugdroid1@chromium.org, May 26 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5f54ca6de628657215c329dbc6bcf2720aad25dd

commit 5f54ca6de628657215c329dbc6bcf2720aad25dd
Author: Sorin Jianu <sorin@chromium.org>
Date: Fri May 26 21:22:07 2017

Clean up the unpack directory for CRLSet component.

In the current implementation of the component updater, the component
installer owns the unpack path of a component and it is responsible
for deleting the unpack path before the execution flow returns to
the component updater during the update of a component.

For most components, this clean up is done in the implementation of
the DefaultComponentInstaller class. However, CRLSet is one of the few
components which is not reusing the DefaultComponentInstaller for
its component installer. As a result, it is leaking the content of
the unpack path after each update cycle.

It is not clear when this regression has been introduced.

BUG= 725671 

Review-Url: https://codereview.chromium.org/2906683002
Cr-Original-Commit-Position: refs/heads/master@{#474700}
Review-Url: https://codereview.chromium.org/2909693004 .
Cr-Commit-Position: refs/branch-heads/3071@{#707}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/5f54ca6de628657215c329dbc6bcf2720aad25dd/chrome/browser/net/crl_set_fetcher.cc

Project Member

Comment 16 by bugdroid1@chromium.org, May 30 2017

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

commit 23edf9fb4b428696ecf6899e3e8e72d149f9eea5
Author: Sorin Jianu <sorin@chromium.org>
Date: Tue May 30 16:55:14 2017

Clean up the unpack directory for CRLSet component.

This is the iOS fix corresponding to:
https://codereview.chromium.org/2906683002

In the current implementation of the component updater, the component
installer owns the unpack path of a component and it is responsible
for deleting the unpack path before the execution flow returns to
the component updater during the update of a component.

For most components, this clean up is done in the implementation of
the DefaultComponentInstaller class. However, CRLSet is one of the few
components which is not reusing the DefaultComponentInstaller for
its component installer. As a result, it is leaking the content of
the unpack path after each update cycle.

It is not clear when this regression has been introduced.

BUG= 725671 

Change-Id: Iba01e7d9210fea4da92718cff833ea1f758935bf
Reviewed-on: https://chromium-review.googlesource.com/517582
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#475567}
[modify] https://crrev.com/23edf9fb4b428696ecf6899e3e8e72d149f9eea5/ios/chrome/browser/net/crl_set_fetcher.cc

Comment 17 by sorin@chromium.org, May 30 2017

Labels: Merge-Request-60
The change for the desktop was merged in M59 and has made the M60 branch cut. Since the code is duplicated in the iOS tree, another changelist was landed on trunk to fix iOS as well. This is a merge request for the corresponding iOS change.
Project Member

Comment 18 by bugdroid1@chromium.org, May 30 2017

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

commit 986198a396c0ce5c05bca498730547a7e1ee919e
Author: Sorin Jianu <sorin@chromium.org>
Date: Tue May 30 18:01:43 2017

Clean up the unpack directory for CRLSet component.

This is the iOS fix corresponding to:
https://codereview.chromium.org/2906683002

In the current implementation of the component updater, the component
installer owns the unpack path of a component and it is responsible
for deleting the unpack path before the execution flow returns to
the component updater during the update of a component.

For most components, this clean up is done in the implementation of
the DefaultComponentInstaller class. However, CRLSet is one of the few
components which is not reusing the DefaultComponentInstaller for
its component installer. As a result, it is leaking the content of
the unpack path after each update cycle.

It is not clear when this regression has been introduced.

BUG= 725671 

Change-Id: Iba01e7d9210fea4da92718cff833ea1f758935bf
Reviewed-on: https://chromium-review.googlesource.com/517582
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#475567}
Review-Url: https://codereview.chromium.org/2909223005 .
Cr-Commit-Position: refs/branch-heads/3071@{#720}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/986198a396c0ce5c05bca498730547a7e1ee919e/ios/chrome/browser/net/crl_set_fetcher.cc

Comment 19 by sorin@chromium.org, May 31 2017

Labels: -OS-Windows OS-All
Labels: TE-Verified-59.0.3071.82
Verified the fix on Windows, Mac and Linux with below after the update of components(CRL Set) there was no junk folder left behind. 


Note : On Mac and Linux it wasn't that straight forward as it's on Windows. 
Project Member

Comment 21 by sheriffbot@chromium.org, May 31 2017

Labels: -Merge-Request-60 Merge-Approved-60
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Thanks for the fix. Please merge to 3112 branch before 4.00 PM PST today(05/31) to make it to next Dev release.
Project Member

Comment 23 by bugdroid1@chromium.org, May 31 2017

Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/25f73c6377429afb1fe14ae9c7f625624feb5c98

commit 25f73c6377429afb1fe14ae9c7f625624feb5c98
Author: Sorin Jianu <sorin@chromium.org>
Date: Wed May 31 20:40:16 2017

Clean up the unpack directory for CRLSet component.

This is the iOS fix corresponding to:
https://codereview.chromium.org/2906683002

In the current implementation of the component updater, the component
installer owns the unpack path of a component and it is responsible
for deleting the unpack path before the execution flow returns to
the component updater during the update of a component.

For most components, this clean up is done in the implementation of
the DefaultComponentInstaller class. However, CRLSet is one of the few
components which is not reusing the DefaultComponentInstaller for
its component installer. As a result, it is leaking the content of
the unpack path after each update cycle.

It is not clear when this regression has been introduced.

BUG= 725671 

Change-Id: Iba01e7d9210fea4da92718cff833ea1f758935bf
Reviewed-on: https://chromium-review.googlesource.com/517582
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#475567}
Review-Url: https://codereview.chromium.org/2913293002 .
Cr-Commit-Position: refs/branch-heads/3112@{#64}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

[modify] https://crrev.com/25f73c6377429afb1fe14ae9c7f625624feb5c98/ios/chrome/browser/net/crl_set_fetcher.cc

Comment 24 by sorin@chromium.org, May 31 2017

This fix is now in 59 and 60 on all platforms.
Cc: sorin@chromium.org
 Issue 700674  has been merged into this issue.

Sign in to add a comment