Closed Bug 1652925 Opened 4 years ago Closed 4 years ago

WebExtensions are allowed to use absurdly large amounts of memory

Categories

(WebExtensions :: General, defect, P1)

Product:
WebExtensions
Component:
General
Version:
79 Branch
Platform:
Desktop
Unspecified
Type:
defect

Tracking

(firefox-esr7884+ fixed, firefox82 wontfix, firefox83 wontfix, firefox84 fixed, firefox85 fixed)

Status:
RESOLVED FIXED
Milestone:
85 Branch
Tracking Flags:
Tracking Status
firefox-esr78 84+ fixed
firefox82 --- wontfix
firefox83 --- wontfix
firefox84 --- fixed
firefox85 --- fixed

People

(Reporter: kael, Assigned: robwu)

References

(Depends on 1 open bug, Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(7 files, 2 obsolete files)

memory-report.json.gz
248.72 KB, application/x-gzip
Details
largest strings
452.69 KB, image/png
Details
infinite-iframe-creation.zip
591.76 KB, application/zip
Details
98877333-776a6b00-243d-11eb-9b14-fcc48d5d3822.png
57.08 KB, image/png
Details
webext-issue-1652925.zip
1.20 KB, application/zip
Details
Bug 1652925 - Fix memory leak of extension Port via conduits
47 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-beta+
RyanVM
: approval-mozilla-release-
jcristau
: approval-mozilla-esr78+
Details | Review
Bug 1652925 - Fix eslint issue in ConduitsChild.jsm on ESR78
47 bytes, text/x-phabricator-request
Details | Review
Attached file memory-report.json.gz —

This may or may not be a bug in the extension in question (uBlock origin) but I was surprised to discover that this behavior was possible at all.

I have uBlock origin enabled for my profile and generally don't have any trouble with it. While glancing through about:memory I discovered a tab I left open for a day or so was now using about 2 gigabytes of memory (almost entirely due to ads and trackers), so I closed it. After Firefox spent a few seconds recovering from this, I fired off GC/CC manually to see whether anything would fail to get flushed out. After doing this, I noticed that the WebExtensions process was using over 2GB of memory, most of it under the 'strings' 'malloc-heap' for uBlock origin. I've attached an anonymized memory report. When I expanded the little summary view before anonymizing erased everything, I didn't see any particularly large string objects, so it seems like it somehow managed to allocate and retain a truly ridiculous number of string objects.

There may be some reasons why this is necessary that I'm not aware of, but I'm surprised that the webextensions process as a whole - or even a single extension - is allowed to use this much memory. I couldn't find any way to knock over this specific extension or restart it even though I know it's not performing any ongoing work, and the GC/CC buttons don't seem to free up any of the memory it's using.

This extension memory usage is also not visible in about:performance at all. According to about:performance my Firefox session is slim and happy, which is very much not the case.

To look into the memory report.

Flags: needinfo?(tomica)

I have been running uBlock Origin since this morning, and about:memory reports ~13.5 MB, with strings consuming ~5.8 MB.

Can you share a non-anonymized report? I can't imagine uBO ever using that amount of memory for strings (especially that uBO is largely TypedArray-based). I would like to find out what are those strings.

This is the only time I've had memory issues with uBO, and it coincided with leaving a nasty page open with uBO disabled that was using ~4GB of heap. Naturally, uBO is not disabled for that page anymore :)

I wasn't able to get a non-anonymized report, once I saved an anonymized one everything in about:memory was anonymized even if I hit "measure" again. Sorry.

When I looked at the sample strings in about:memory they were all data: urls but none of the individual strings were large.

OK, I reproduced this. My repro steps:

  1. Open https://www.vice.com/en_us/article/v7gzdd/why-professional-gamers-are-getting-loans-from-the-government in a tab
  2. Disable uBlock Origin for the tab if it is already enabled, refresh
  3. Leave the tab open
  4. Come back later

After doing this the tab and its content process in general are using a ton of RAM and so is uBlock origin. I'm going to email you a non-anonymized report, I turned on verbose mode as well.

Attached image largest strings —

In case it didn't make it into the saved memory report, here are the largest strings in the WebExtensions process in verbose mode.

Thanks for the repro steps. I had a look at the verbose memory report and I have to admit I am at a lost about all those data: URI strings -- these are not created by uBO, so I have to find out where they come from. I will investigate using your steps.

Question re. repro steps:

  • do I have to leave the tab active and/or visible, or can it be inactive while another tab is visible?
  • did you scroll down or just left the tab as is untouched after reload?
  • when you "come back later", roughly what delay are we talking about? minutes? hours?
  • did you change about:config entries?
  • is fission.autostart set to true in about:config?

Those URLs look like favicons, which are exposed through the favIconUrl property of the tabs API. This property is only present if an extension has the tabs permission. To see if it is potentially causing the bug, fork uBlock Origin and remove its tabs permission.

The 9140 copies of a 44441-length string looks odd. If you open that data:-URL, you may get a hint on the source of the leak.
I can just see enough of the data-URL in the screenshot to see its metadata (It's a ICO file containing 5 square images of sizes 16, 24, 32, 48 and 64); the rest of the data has been truncated.

(In reply to Rob Wu [:robwu] from comment #7)

Those URLs look like favicons, which are exposed through the favIconUrl property of the tabs API. This property is only present if an extension has the tabs permission. To see if it is potentially causing the bug, fork uBlock Origin and remove its tabs permission.

The 9140 copies of a 44441-length string looks odd. If you open that data:-URL, you may get a hint on the source of the leak.
I can just see enough of the data-URL in the screenshot to see its metadata (It's a ICO file containing 5 square images of sizes 16, 24, 32, 48 and 64); the rest of the data has been truncated.

Could the strings be getting associated with the wrong extension? I have Tree Style Tab installed, which I would expect to be interacting with favicons. However, disabling + re-enabling ublock frees all the strings, so they seem to be in its heap...

EDIT: I don't know how to get the full data URL. It is truncated in the verbose memory report.

(In reply to rhill@raymondhill.net from comment #6)

Thanks for the repro steps. I had a look at the verbose memory report and I have to admit I am at a lost about all those data: URI strings -- these are not created by uBO, so I have to find out where they come from. I will investigate using your steps.

Question re. repro steps:

  • do I have to leave the tab active and/or visible, or can it be inactive while another tab is visible?

I opened it up and let it load, then left it inactive and it kept running its JS or whatever in the background.

  • did you scroll down or just left the tab as is untouched after reload?

I think I scrolled down a bit. It typically has ads at the top of the viewport though.

  • when you "come back later", roughly what delay are we talking about? minutes? hours?

I think at least an hour is enough. You might not need that long, since it eats an absurd amount of memory.

disabling + re-enabling ublock frees all the strings

Does closing the tab cause the memory to be freed as well? (allowing some time for the garbage collector to kick in)

After trying many scenarios to reproduce (including using a VPN to get U.S. ads), and since I still could not reproduce, I went to look where in uBO there could be URLs kept for a long time. In your verbose memory snapshot, I see many entries under one of the strings section in the WebContent of the article, actually over 500 <!DOCTYPE html>... entries:

0.24 MB (00.02%) ++ string(length=103, copies=911, "https://www.vice.com/en_us/article/v7gzdd/why-professional-gamers-are-getting-loans-from-the-government")
0.19 MB (00.01%) ++ string(length=1296, copies=48, "data://image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAJCAMAAAAM9FwAAAAAGXRFWHRTb2Z0d2FyZQBBZG9iZSBJbWFnZVJlYWR5ccllPAAAAyNpVFh0WE1MOmNvbS5hZG9iZS54bXAAAAAAADw/eHBhY2tldCBiZWdpbj0i77u/IiBpZD0iVzVNME1wQ2VoaUh6cmVTek5UY3prYzlkIj8+IDx4OnhtcG1ldGEgeG1sbnM6eD0iYWRvYmU6bnM6bWV0YS8iIHg6eG1wdGs9IkFkb2JlIFhNUCBDb3JlIDUuNi1jMTQwIDc5LjE2MDQ1MSwgMjAxNy8wNS8wNi0wMTowODoyMSAgICAgICAgIj4gPHJkZjpSREYgeG1sbnM6cmRmPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5LzAyLzIyLXJkZi1zeW50YXgtbnMjIj4gPHJkZjpEZXNjcmlwdGlvbiByZGY6YWJvdXQ9IiIgeG1sbnM6eG1wPSJodHRwOi8vbnMuYWRvYmUuY29tL3hhcC8xLjAvIiB4bWxuczp4bXBNTT0iaHR0cDovL25zLmFkb2JlLmNvbS94YXAvMS4wL21tLyIgeG1sbnM6c3RSZWY9Imh0dHA6Ly9ucy5hZG9iZS5jb20veGFwLzEuMC9zVHlwZS9SZXNvdXJjZVJlZiMiIHhtcDpDcmVhdG9yVG9vbD0iQWRvYmUgUGhvdG9zaG9wIENDIChNYWNpbnRvc2gpIiB4bXBNTTpJbnN0YW5jZUlEPSJ4bXAuaWlkOjQ2QjUwODI3MUI1NTExRTk5RjE3QTJDRDlCQzU1MDg0IiB4bXBNTTpEb2N1bWVudElEPSJ4bXAuZGlkOjQ2QjUwODI4MUI1NTExRTk5RjE3QTJDRDlCQzU1MDg0Ij4gPHhtcE1NOkRlcml2ZWRGcm9tIHN0UmVmOmluc3RhbmNlSUQ9InhtcC5paWQ6NDZCNTA4MjUxQjU1MTFFOTlGMTdBMkNEOUJ" (truncated))
0.17 MB (00.01%) ++ string(length=1296, copies=44, "data://image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAMAAAAoyzS7AAAAGXRFWHRTb2Z0d2FyZQBBZG9iZSBJbWFnZVJlYWR5ccllPAAAAyNpVFh0WE1MOmNvbS5hZG9iZS54bXAAAAAAADw/eHBhY2tldCBiZWdpbj0i77u/IiBpZD0iVzVNME1wQ2VoaUh6cmVTek5UY3prYzlkIj8+IDx4OnhtcG1ldGEgeG1sbnM6eD0iYWRvYmU6bnM6bWV0YS8iIHg6eG1wdGs9IkFkb2JlIFhNUCBDb3JlIDUuNi1jMTQwIDc5LjE2MDQ1MSwgMjAxNy8wNS8wNi0wMTowODoyMSAgICAgICAgIj4gPHJkZjpSREYgeG1sbnM6cmRmPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5LzAyLzIyLXJkZi1zeW50YXgtbnMjIj4gPHJkZjpEZXNjcmlwdGlvbiByZGY6YWJvdXQ9IiIgeG1sbnM6eG1wPSJodHRwOi8vbnMuYWRvYmUuY29tL3hhcC8xLjAvIiB4bWxuczp4bXBNTT0iaHR0cDovL25zLmFkb2JlLmNvbS94YXAvMS4wL21tLyIgeG1sbnM6c3RSZWY9Imh0dHA6Ly9ucy5hZG9iZS5jb20veGFwLzEuMC9zVHlwZS9SZXNvdXJjZVJlZiMiIHhtcDpDcmVhdG9yVG9vbD0iQWRvYmUgUGhvdG9zaG9wIENDIChNYWNpbnRvc2gpIiB4bXBNTTpJbnN0YW5jZUlEPSJ4bXAuaWlkOjIxNEVGMEY4MUI1NTExRTk5RjE3QTJDRDlCQzU1MDg0IiB4bXBNTTpEb2N1bWVudElEPSJ4bXAuZGlkOjIxNEVGMEY5MUI1NTExRTk5RjE3QTJDRDlCQzU1MDg0Ij4gPHhtcE1NOkRlcml2ZWRGcm9tIHN0UmVmOmluc3RhbmNlSUQ9InhtcC5paWQ6Qzk3QTI5Q0YxQjREMTFFOTlGMTdBMkNEOUJ" (truncated))
0.13 MB (00.01%) ++ string(length=45451, copies=1, "<!doctype html><html><head><script>var inDapIF=true,inGptIF=true;</script></head><body leftMargin="0" topMargin="0" marginwidth="0" marginheight="0"><script>window.dicnf = {};</script><script data-jc="41" data-jc-version="r20200713">(function(){/*  Copyright The Closure Library Authors. SPDX-License-Identifier: Apache-2.0 */ 'use strict';window.viewReq=[];window.vu=b=>{{const a=new Image;a.src=b.replace("&amp;","&");viewReq.push(a)}};}).call(this);</script><script>vu("https://securepubads.g.doubleclick.net/pcs/view?xai//x3dAKAOjssXLJX2DSnWoG2YXDZwNU7ZHa1GDzNmaJpjvVupDn_eaZtCoMBkp3oT6kgrtkTJ3ZdBWH0pbnE7UQ2JT4P5fOqp6VXwVG5xVevMArhH1d-Su1L8KkEQSlBi2zjFsFpav55MxFuxQT4ImF7cV3w4KbuaeMJEyxWlyS8BxyvqXvwzcaMgXsJr6DnZkljU3TNjj5oWur2aQT9BiOaF-oBUvsS0geac5qR9kRq_xMj8gYGFh0uHLVHK-atfn_EM0pRE0MBLFVguJzL_3ZY65-kovzU7//x26sig//x3dCg0ArKJSzAL3CxTRlnNxEAE//x26urlfix//x3d1//x26adurl//x3d")</script><div class="GoogleActiveViewInnerContainer"style="left:0px;top:0px;width:100%;height:100%;position:fixed;pointer-events:none;z-inde" (truncated))
0.07 MB (00.01%) ++ string(length=1408, copies=18, "_scid=f14327cf-9c14-4e91-9407-b04f75d8fbfd; GED_PLAYLIST_ACTIVITY=W3sidSI6IjNGNzciLCJ0c2wiOjE1OTQ3Mzg4MDYsIm52IjoxLCJ1cHQiOjE1OTQ3Mzg4MDQsImx0IjoxNTk0NzM4ODA1fV0.; viv={"ids":[]}; campaign_viv={"campaign":{}}; _cb_ls=1; X-GeoIP-Region-Code=CA; _sctr=1|1594623600000; ccpaUUID=7bcf3b4f-0785-4d02-92ac-403fee7df5cf; __gads=ID=93568bed3fb3fe53:T=1594708734:S=ALNI_MamJNKwwu_FtXLvSX3PR9ytRCUWUw; __qca=P0-443262496-1594708736759; OB-USER-TOKEN=32b3d753-d358-4eff-8d98-062c5f96848f; X-GeoIP-Country-Code=US; X-Vice-Split-Testing=2020-07-15a:web-next; __auc=d95a1c311734c0c2273d9b87b6c; _cb=DzTmx1p-BNUoxsnh; _chartbeat2=.1532049415350.1594941755744.1111001010011111.pcmr6FpMiDBp04dDpFF-DC7-uXm.2; _ga=GA1.2.1244563756.1594708731; _gid=GA1.2.5658847.1594941700; _gaClientId=1244563756.1594708731; _vice_from_GDPR_region=true; _fbp=fb.1.1594669795405.2135264368; kppid_managed=kppidff_NhwRNNCS; consentUUID=be54131d-ce8f-4364-bfec-613410ce23bf; _sp_krux=true; _sp_enable_dfp_personalized_ads=true; dnsDisplayed=true; ccpaApplies=t" (truncated))
0.03 MB (00.00%) ++ string(length=11998, copies=1, "<!-- Wrapper for Vice Media, generated on 2020-05-26T16:51:11-04:00 -->/n    <script class='clrm-cw-head' type='text/javascript'>/n       ;(function (w, N, b) {/n           w[N] = {/n               a:'21810178367',A:'/16916245/oo_web/vice/games/article',h:'250', w:'970', c:'138314085012',o:'2680689999',ad:'4858582973',l:'5396266553',p:'15916365',k:{"ad_group":["ad_opt"],"aid":["why-professional-gamers-are-getting-loans-from-the-government"],"brand_name":["vice"],"country":["en_us"],"gs_cat":["cg_google_au","gs_interest_frequent_travelers","gs_interest_male","gs_interest_online_shoppers","gs_business","gs_entertain","gv_safe","gv_safe_core","middle_east_safe","pos_safe_low","pos_uk_marriott_english_brand_safety","cg_uksafe","neg_toyota","gs_entertain_vidgames","gs_sport","gs_business_misc","neg_cg_courvoisier","gs_covid19","neg_cg_apple","gs_sport_basketball"],"iom":["970x250_15"],"ix_id":["_yd2u4pZ2"],"keywords":["esports","pandemic","overwatch-league","ppp-loans"],"pagetype":["short-form"],"pos":["first"],"" (truncated))
0.02 MB (00.00%) ++ string(length=2846, copies=5, "<div id="te-clr1-81e5f3b0-0396-4470-9fd1-7a707528d8b5-itl" style="position: absolute; margin: 0pt auto; padding: 0px; width: 300px; height: 250px; border: 1px solid rgb(204, 204, 204); display: block; background: none repeat scroll 0% 0% rgb(255, 255, 255); overflow: hidden; line-height: 12px; font-size: 11px; font-family: arial,helvetica,clean,sans-serif; text-align: left; z-index: 1001;"> /n   <div class="closeTag" style="float: right; padding: 5px;"><a style="cursor: pointer;" alt="close" onclick="javascript:truste.ca.hideoverlay(te_clr1_e53312ac_2535_459b_abbf_0f92d58e2cde_bi)">[close]</a></div> /n   <div class="mainRolloverSection"> /n      <div style="padding: 0px 3px;">/n        <a onclick="truste.ca.interstitial_click('001', te_clr1_e53312ac_2535_459b_abbf_0f92d58e2cde_bi )" href="https://www.thetradedesk.com/" target="_blank"  style="display: block; padding-top: 5px; border-top: 1px solid #bbb;">/n        <img src="https://choices.trustarc.com/get?name=theTradeDesk-LogoLockup-RGB.png" style="vertica" (truncated))
0.02 MB (00.00%) ++ string(length=3, copies=833, "300")/gc-heap
0.02 MB (00.00%) ++ string(length=1831, copies=4, "_scid=f14327cf-9c14-4e91-9407-b04f75d8fbfd; GED_PLAYLIST_ACTIVITY=W3sidSI6IjNGNzciLCJ0c2wiOjE1OTQ3Mzg4MDYsIm52IjoxLCJ1cHQiOjE1OTQ3Mzg4MDQsImx0IjoxNTk0NzM4ODA1fV0.; viv={"ids":[]}; campaign_viv={"campaign":{}}; _cb_ls=1; X-GeoIP-Region-Code=CA; _sctr=1|1594623600000; ccpaUUID=7bcf3b4f-0785-4d02-92ac-403fee7df5cf; __gads=ID=93568bed3fb3fe53:T=1594708734:S=ALNI_MamJNKwwu_FtXLvSX3PR9ytRCUWUw; __qca=P0-443262496-1594708736759; OB-USER-TOKEN=32b3d753-d358-4eff-8d98-062c5f96848f; X-GeoIP-Country-Code=US; _vice_from_GDPR_region=true; kppid_managed=kppidff_NhwQ3c1V; _cb=DzTmx1p-BNUoxsnh; _chartbeat2=.1532049415350.1594941699745.1111001010011111.pcmr6FpMiDBp04dDpFF-DC7-uXm.1; _cb_svref=null; _ga=GA1.2.1244563756.1594708731; _gid=GA1.2.5658847.1594941700; _gat=1; _gat_optimizeTracker=1; _gaClientId=1244563756.1594708731; cto_bundle=PSQNL19NeWhRemRIeEFad1hJdkMwNmpVUiUyRm5YJTJGaUdqdWtjU1daRDRtbXRvWlVRZUxNNTJYbDV4WUFMRlglMkJ2NmtrN2U0TUQ3ZHdFY21YZFp6S21HT2ZtQ1N3RnlTck5yUHNieTB0WjBUa0J1RFVOcDdjMUtBSmtMdU5wMDJqbSUyQjh6dmdvQU" (truncated))
0.02 MB (00.00%) ++ string(length=4471, copies=1, "<!DOCTYPE html><html><head></head><body style="margin: 0px; padding: 0px;"><script>window.sapl_callback = function(args) {try {window.top["sapl_1594941759481"](args);} catch (e) { throw "Could not locate the global syndication callback to pass ad fill message. " + e; };};</script><script src="//ad-cdn.technoratimedia.com/01/47/88/uat_88471.js?ad_size=300x250&callback=sapl_callback&seat=display&idx=250"></script></body></html><div id="uat6681"></div><iframe id="tn8UserSync" height="0" width="0" frameborder="0" style="visibility: hidden; display: none" src="https://ad-cdn.technoratimedia.com/html/usersync.html"></iframe><script type="text/javascript" src=https://adtag.technoratimedia.com/adserv_88471.js?ad_size=300x250&callback=sapl_callback&seat=display&idx=250&referrer=https%3A//www.vice.com/en_us/article/v7gzdd/why-professional-gamers-are-getting-loans-from-the-government&disp=none&tmiv=1&tcb=0.13658866036408446&abv=BTF></script><!-- Creative 9ohh0kue served by the TTD platform via Synacor --><img src="http" (truncated))
[...]

This made me look at the code in uBO dealing with embedded iframes in a given main document, and I did find a scenario where in a long-lived page, uBO's internal dictionary of iframes for a given document can grow unbound, and this happens because there is no event in the WebExtensions API itself which can tell extensions when an iframe is removed. I suppose such unbound growth would manifest itself more readily when uBO is not blocking anything on a ad-ladden page.

So I created a minimal repro case (attachment) for this (need a local server), and after letting it execute for a while, I can get a memory measurement that ressembles what I see in your verbose memory measurement:

0.05 MB (00.07%) ++ string(length=8078, copies=4, "https://gorhill.github.io/uBlock/tests/plagiomnium_affine_laminazellen.jpeg?_=894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098942563856318109894256385631810989425638563181098" (truncated))
0.05 MB (00.06%) ++ string(length=8078, copies=3, "https://gorhill.github.io/uBlock/tests/plagiomnium_affine_laminazellen.jpeg?_=346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633469580103802363346958010380236334695801038023633" (truncated))
0.05 MB (00.06%) ++ string(length=8078, copies=3, "https://gorhill.github.io/uBlock/tests/plagiomnium_affine_laminazellen.jpeg?_=691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266918568163779626691856816377962669185681637796266" (truncated))
0.04 MB (00.05%) ++ string(length=8078, copies=3, "https://gorhill.github.io/uBlock/tests/plagiomnium_affine_laminazellen.jpeg?_=578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695789790733482869578979073348286957897907334828695" (truncated))
0.03 MB (00.04%) ++ string(length=17, copies=1394, "gorhill.github.io")/gc-heap
0.03 MB (00.04%) ++ string(length=8078, copies=2, "https://gorhill.github.io/uBlock/tests/plagiomnium_affine_laminazellen.jpeg?_=329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073299046738191007329904673819100732990467381910073" (truncated))
0.03 MB (00.04%) ++ string(length=8078, copies=2, "https://gorhill.github.io/uBlock/tests/plagiomnium_affine_laminazellen.jpeg?_=419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874197066669778887419706666977888741970666697788874" (truncated))
0.03 MB (00.04%) ++ string(length=8078, copies=2, "https://gorhill.github.io/uBlock/tests/plagiomnium_affine_laminazellen.jpeg?_=533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365338312755020536533831275502053653383127550205365" (truncated))
0.03 MB (00.04%) ++ string(length=8078, copies=2, "https://gorhill.github.io/uBlock/tests/plagiomnium_affine_laminazellen.jpeg?_=633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856335358226737785633535822673778563353582267377856" (truncated))

I emphasized "ressembles" because I can't reproduce what I see in your memory measurement when I use a data: URIs for the source property on the iframes. When I use a data: URI, the code path where uBO put the URL in its iframe dictionary is never reached. So I still don't know the exact issue behind the memory leak you see.

Nevertheless I patched the code on uBO's side to avoid unbound iframe dictionary growth. This will be in the next dev build of uBO and maybe you could give it a try -- I will post here when it's ready.

Unzip into a folder and have a local server to that folder and load index.html in your browser.

uBO 1.28.5b2 contains code to prevent unbound growth of subframe dictionary in long-lived tabs:
https://github.com/gorhill/uBlock/releases/tag/1.28.5b2

The dev build can be installed as a replacement of the stable build (and vice versa) without having to reinstall and lose settings.

:kael, can you see if it makes a difference?

Thank you Raymond for the investigation and action. I don't see indication that this is a general extensions framework problem, so moving to Dev Outreach, and setting priority since it's already being worked on.

(In reply to Katelyn Gadd (:kael) from comment #0)

but I'm surprised that the webextensions process as a whole - or even a single extension - is allowed to use this much memory. I couldn't find any way to knock over this specific extension or restart it even though I know it's not performing any ongoing work, and the GC/CC buttons don't seem to free up any of the memory it's using.

We don't impose arbitrary memory limits even on plain public web pages, and those are much less trustworthy than extension that users need to explicitly install before using. One's person's "sensible limit" is often way lower than other's "workflow" (cue xkcd).

The issue often is that extensions are basically long-lived web pages, so it's harder for users to detect they are the cause, or how to solve it (than closing a misbehaving tab). Some of those issues are going to be solved by Manifes v3 and background service workers, but there's no universal silver bullet for something like this, and reporting problems to addon authors is the the best solution we have.

Severity: -- → S3
Component: General → Developer Outreach
Flags: needinfo?(tomica)
Priority: -- → P2

(In reply to Tomislav Jovanovic :zombie from comment #13)

We don't impose arbitrary memory limits even on plain public web pages, and those are much less trustworthy than extension that users need to explicitly install before using. One's person's "sensible limit" is often way lower than other's "workflow" (cue xkcd).

The issue often is that extensions are basically long-lived web pages, so it's harder for users to detect they are the cause, or how to solve it (than closing a misbehaving tab). Some of those issues are going to be solved by Manifes v3 and background service workers, but there's no universal silver bullet for something like this, and reporting problems to addon authors is the the best solution we have.

Should the memory usage in this case have shown up in about:performance? It didn't.

I would reasonably expect so, but I'm not that familiar with what about:performance is supposed to measure exactly. If you have a reliable STR, please file a bug in Toolkit:Pefromance Monitoring, so those folks can check it out.

I have Tree Style Tab installed, which I would expect to be interacting with favicons

There are many instances of faviconUrl in Tree Style Tabs source code. Can you reproduce the issue if you disable all other extensions and keep only uBO enabled?

Currently after maybe one hour, the explicit memory for the WebContent for the web page climbed to 680MB, and uBO at 22MB with strings taking 2.7MB. I am still unable to reproduce.

I tested for a while with TST disabled and couldn't reproduce the leak, but I also can't reproduce with it turned back on. I think Vice must have rotated their ad buys or kicked the problem ad out...

My guess is you're correct about it being the subframe dictionary, since the page clearly was doing nasty things with subframes. TST making it worse would make sense if it was doing something in response to the frame/subframe navigation (fetching a unique copy of the favicon every time? etc) and then uBO was getting convinced to erroneously store that favicon in memory. I expect your fix will put a stop to that. If I end up seeing this again I'll definitely test with TST disabled now that I know it could be interacting with uBO in this way - I was surprised to learn that extensions could meddle with each other like that but I can understand how it would be the case.

I found a difference scenario in which I can systematically reproduce data:image/x-icon entries in uBO's memory report. There is no need to disable uBO.

  • New Firefox session with only one pinned tab, about:addons
  • Have uBO as the only extension, ensure all up to date lists to avoid spurious memory usage
  • Go to https://news.ycombinator.com/
  • Middle-click every news entry link on the page
  • Click "More" at the bottom, repeat above step
  • Click "More" at the bottom, repeat above step
  • So now there should be around 90 tabs opened
  • Be sure "Restore previous session" is enabled
  • At this point, there should already be data:image/x-icon for uBO in about:memory, but I prefer to restart for a clean slate
  • Quit Firefox
  • Restart Firefox
  • Leave idle for 1-2 minutes
  • New tab to about:memory, click "Measure" (forcing CC/GC makes no difference)
  • Go to WebExtensions section
  • Under uBO's js-zones/strings, expand the list of tiny strings
  • Result: entries with data:image/x-icon;base64,... (around 10-15 on my side)

Since uBO does not store or even access favIconUrl, the challenge at this point is to identify why are these strings in uBO's heap. There are not many calls to tabs.get/tabs.query in uBO's code base, and so far I have been unable to identify anything wrong where these calls appear, the tab object returned by tabs.get/tabs.query is never stored, it's only used locally and thus should be GC'ed when exiting the scope.

Edit 1: I notice that the set of data:image/x-icon is always the same each time the same session is reloaded. The top one consuming most memory is the favicon from http://www.pyrunner.com/weblog/2016/05/26/compressed-sensing-python/, a data URI of 132,929 characters. The URL for the favicon as per view-source is http://www.pyrunner.com/static/favicon.ico.

Edit 2: Given the above findings, I reduced the repro steps to only two tabs:

  • about:addons (I pinned it in my case)
  • http://www.pyrunner.com/weblog/2016/05/26/compressed-sensing-python/

Relaunching the browser with the two tabs above will cause the stray data:image/x-icon entry to occur if and only if upon restore the about:addons is the default one and the other tab -- the one "leaking" a data URI -- is not activated. I will keep trying to narrow.

Edit 3: Observation, if I add the instruction tab.favIconUrl = ''; inside this loop, the stray data URI disappears in the revised scenario above. So far, my understanding is that the WebExtensions framework is using the extension's heap to allocate memory for the favIconUrl upon calling tabs.query(), but it's never released afterward even though the owner Tab object goes out of scope.

Edit 4: Observation, it appears this has to do with calling tabs.executeScript() on a tab which discarded property is true. If I call tabs.executeScript() only for when Tab.discarded !== true, the data URI entry disappears in the above scenario.

Depends on: 1653876

Raymond, did you check whether the initializeTabs method runs to completion? At line 78 you're awaiting promises. At line 83 you're referencing the tabs array, so all data within is forced to be kept in memory until that part of the code has been reached.
I created a minimal reproduction based on your code and found that it does not run to completion - reported as bug 1653876.

Your code is designed to run content scripts in existing tabs, before the extension was loaded.
Unlike Chrome, Firefox already runs content scripts from manifest.json in existing tabs. Can't you work around this bug by not running initializeTabs on Firefox?

If you can't do that, another work-around to avoid the unbounded memory leak is to map the tab objects to a list of tabIds. There will still be some unclaimed memory, but it would be negligible.

PS: I'm not sure if your finding is comparable to the initial bug report. The OP suggested that the memory leak accumulated over time, whereas your STR only causes issues at the start of the extension.

Raymond, did you check whether the initializeTabs method runs to completion?

Yes, I put a breakpoint after Promise.all() and it was never taken -- I wasn't sure whether this was a debugger quirk, but given your findings I guess the Promise.all() just never resolved.

Can't you work around this bug by not running initializeTabs on Firefox?

The initialization code just inject a small script which will check whether the content scripts have already been injected, and if so, whether the bootstrap task has executed. For now the code change I made is to mind discarded property. The content scripts are designed to be idle when injected before the extension is ready, so initializeTabs() is necessary to "un-idle" the bootstrap code so they can start to do what they are meant to do.

map the tab objects to a list of tabIds.

Yes, probably best to do this regardless anyways.

I'm not sure if your finding is comparable to the initial bug report. The OP suggested that the memory leak accumulated over time, whereas your STR only causes issues at the start of the extension.

I found another scenario causing a data: URI leak in uBO's heap:

  • Configure Firefox (78.0.2) such that just no web page is loaded at launch (other than chrome pages)
  • Navigate to about:preferences#privacy and under "Cookies and Site Data", click "Clear Data..." button and ensure that "Cached Web Content" is empty.
    • For convenience, we can keep this page opened across launches
  • Quit Firefox, launch Firefox
  • In a new tab, navigate to http://www.pyrunner.com/weblog/2016/05/26/compressed-sensing-python/
  • Once the page is loaded, click the link "Mathematics" at the top
  • In a new tab, navigate to about:memory, click "Measure"
  • Result, a data:image/x-icon;base64,... in uBO's js-zone => strings => x tiny:
    string(length=132929, copies=1, "data:image/x-icon;base64,AAAB...

The "Clear Data..." step is important, otherwise the repro steps become unreliable without it. I will keep investigating to figure out what code in uBO is involved in that one scenario.

Edit 1: If I comment out this piece of code in uBO, which is to add a browser.windows.onFocusChanged event listener, the data: URI leak (as per above repro steps) disappears.

Edit 2: Unfortunately, I now lost the ability to reproduce as per above steps. Shortly before I had added console.log to uBO's windows.onFocusChanged listener to verify that the promise was correctly resolved, and eventually after a few back and forth in the code I can no longer reproduce with the latest official dev build of uBO, which I used in the original repro steps above.

Comment 21 sounds like an intermittent issue. To make sure that it's really a leak, opposed to something that's waiting to be garbage-collected, press the GC+CC buttons (a couple of times) or the "Minimize memory usage button" (the latter has more side effects).

I can still reproduce sporadically but I am unable to identify sure repro steps. For now my about:memory reports:

2.79 MB (05.52%) ++ string(length=132929, copies=11, "data:image/x-icon;base64,AAAB...

And I did click many times on GC/CC, then closing the about:memory tab and waiting some more to try again, and the entry won't go away. In the main process I also see:

2.05 MB (01.02%) -- string(length=132929, copies=12, "data:image/x-icon;base64,AAAB...

This happened after I was trying to find steps to reproduce, which did not involve clicking any link, but rather involve opening a new tab to http://www.pyrunner.com/weblog/2016/05/26/compressed-sensing-python/, then from there trying different actions, such as clearing cache then dragging the URL in the tab to a new tab, opening a new tab, closing the old http://www.pyrunner.com/weblog/2016/05/26/compressed-sensing-python/ tab, switching to the new http://www.pyrunner.com/weblog/2016/05/26/compressed-sensing-python/ tab, bringing up uBO's popup panel, then opening about:memory in the empty tab to find out if the leak showed up. It's a difficult case to crack but it does indeed look like some favicon data: URIs are leaking, somehow.

Found simpler repro steps, quite different than above, but hopefully underlying issue -- yet to be identified -- is the same:

  • Navigate to http://www.pyrunner.com/weblog/2016/05/26/compressed-sensing-python/ in a new tab
  • Launch uBO's element picker from either the popup panel, or keyboard shortcut -- I prefer keyboard shortcut since this involves executing less code in order to generate the data: URI leak
  • Press Escape key to exit element picker
  • Result: data: URI leak in uBO's heap

The code to launch element picker is:
https://github.com/gorhill/uBlock/blob/e200d9c31c29a96a3806c98596741d038346a990/src/js/ublock.js#L417-L439

I commented out the last instruction, vAPI.tabs.select(tabId);, but this made no difference. I am unable to see anything wrong with the rest of the code.

Edit 1: Just to confirm, if I remove both calls to tabs.executeScript() in the code linked above, the data: URI leak disappears. So I think this narrows where to look at.

I can still reproduce with the steps in comment #24, I launched the element picker a couple of times, Firefox 82.0.3, filtering output of about:memory on x-icon -- I get this after closing the visited tabs and clicking GC, CC and Minimize memory multiple times:

Main Process (pid 12335)
Explicit Allocations

0.30 MB (100.0%) -- explicit
└──0.30 MB (100.0%) -- js-non-window/zones/zone(0x7f39a0f18000)/strings
   ├──0.25 MB (84.39%) -- string(length=132929, copies=1, "data:image/x-icon;base64,AAABAAUAgIAAAAEAIAAoCAEAVgAAAEBAAAABACAAKEIAAH4IAQAwMAAAAQAgAKglAACmSgEAICAAAAEAIACoEAAATnABABAQAAABACAAaAQAAPaAAQAoAAAAgAAAAAABAAABACAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMz" (truncated))
   │  ├──0.25 MB (84.38%) ── malloc-heap/two-byte
   │  └──0.00 MB (00.01%) ── gc-heap/two-byte
   ├──0.02 MB (07.80%) -- string(length=10061, copies=2, "data:image/x-icon;base64,AAABAAEAAAAAAAEAIABRHQAAFgAAAIlQTkcNChoKAAAADUlIRFIAAAEAAAABAAgGAAAAXHKoZgAACkFpQ0NQSUNDIFByb2ZpbGUAAEgNnZZ3VFPZFofPvTe90BIiICX0GnoJINI7SBUEUYlJgFAChoQmdkQFRhQRKVZkVMABR4ciY0UUC4OCYtcJ8hBQxsFRREXl3YxrCe+tNfPemv3HWd/Z57fX2Wfvfde6AFD8ggTCdFgBgDShWBTu68FcEhPLxPcCGBABDlgBwOFmZgRH+EQC1Py9PZmZqEjGs/buLoBku9ssv1Amc9b/f5EiN0MkBgAKRdU2PH4mF+UClFOzxRky/wTK9JUpMoYxMhahCaKsIuPEr2z2p+Yru8mYlybkoRpZzhm8NJ6Mu1DemiXho4wEoVyYJeBno3wHZb1USZoA5fco09P4nEwAMBSZX8znJqFsiTJFFBnuifICAAiUxDm8cg6L+TlongB4pmfkigSJSWKmEdeYaeXoyGb68bNT+WIxK5TDTeGIeEzP9LQMjjAXgK9vlkUBJVltmWiR7a0c7e1Z1uZo+b/Z3x5+U/09yHr7VfEm7M+eQYyeWd9s7KwvvRYA9iRamx2zvpVVALRtBkDl4axP7yAA8gUAtN6c8x6GbF6SxOIMJwuL7OxscwGfay4r6Df7n4Jvyr+GOfeZy+77VjumFz+BI0kVM2VF5aanpktEzMwMDpfPZP33EP/jwDlpzcnDLJyfwBfxhehVUeiUCYSJaLuFPIFYkC5kCoR/1eF/GDYnBxl+nWsUaHVfAH2FOVC4SQfIbz0AQyMDJG4/egJ961sQMQrIvrxorZGvc48yev7n+h8LXIpu4UxBIlPm9gyPZHIloiwZo9+EbMECEpAHdKAKNIEuMAIsYA0cgDNwA94gAISASBADlgMuSAJpQASyQT7YAApBMdgBdoNqcADUgXrQBE6CNnAGXARXwA1wCwyAR0AKhsFLMAHegWkIgvAQFaJBqp" (truncated))
   │  ├──0.02 MB (07.79%) ── malloc-heap/latin1
   │  └──0.00 MB (00.02%) ── gc-heap/latin1
   └──0.02 MB (07.80%) -- string(length=8717, copies=2, "data:image/x-icon;base64,AAABAAIAEBAAAAEAIAAoBQAAJgAAACAgAAABACAAKBQAAE4FAAAoAAAAEAAAACAAAAABACAAAAAAAAAFAAAAAAAAAAAAAAAAAAAAAAAAOlZR3TCLa/8hcVL/Fk04/yFZRv8xlXD/RLGP/zmnhP81n3v/MJdx/zCCZv8zb2D/RKCJ/07Irv9MzbD/S6uR/Tyeff8onG//GnRP/x5YQf8uo3X/Q7yX/2Dexv9e3cb/WNrC/0nLrf9Fyqj/SdOx/0q/pv9Hnoz/Tcuw/06zlv89sIf/IY1i/yFuUP8wo3X/ObaN/1TTuf9U07v/UNO5/0zUtv9Q2bz/UdO5/0/Qtf9O1Ln/VNm9/0++p/9OtJn+PXtp/yStdf8qf17/OLmL/0/Ns/9U2cL/TdK3/zB/bf82fG3/NoZ2/0e3oP9R0rb/T861/1DYvP9QtaH/T7Wa/wAAAAAyemH/MraG/03kvv9W4Mf/R7yi/yprW/8/n4r/U9m9/0K9oP9Fspz/UtK5/0/Yu/9W2b//Usmx/1DOrf8AAAAASFVYXDp0Y/5JsZr/RqmT/zODbP9At5j/S9a1/0jKrP9Tv6n/TLSg/1DSuf9Qw67/VMOt/1jTvP9XkYjaAAAAAEKKc/ovpnL/Mpxy/0m2nf9Y4MP/UMqw/0q3n/9hzLr/Zr2u/0aSf/8/l4H/PY93/y5+YP8paUr/PEI/WAAAAABTkoPzP8GM/y2hb/9Y2br/N5B//zSWgv9Z38P/Tsux/0Kxlv87n4L/NYxw/y13W/8ve13/LJJr/zmPdv0AAAAAVIF600nOm/8wrHn/PLmc/0cyN/9hcm7/SNu+/1DLsv9Z38X/T9e+/0nQt/9g69L/WNnC/1//5P9btKj3AAAAAE1eXllRx57/NbWB/zm3mP94ybz/ue7k/0PFq/9W1rz/Sc2y/zAxMP9MXVr/UeDC/0zMsv9o/+D/XIJ+uwAAAAAAAAAAVZqH80Tan/9Bt5T/Td7D/0bOs/9XzL" (truncated))
      ├──0.02 MB (07.79%) ── malloc-heap/latin1
      └──0.00 MB (00.02%) ── gc-heap/latin1


extension (pid 12496)
Explicit Allocations

1.52 MB (100.0%) -- explicit
└──1.52 MB (100.0%) -- window-objects/top(moz-extension://6e7ef809-f34a-42f1-82af-df94210ceaa2/background.html, id=31)/js-zone(0x7f974f355000)/strings/string(length=132929, copies=6, "data:image/x-icon;base64,AAABAAUAgIAAAAEAIAAoCAEAVgAAAEBAAAABACAAKEIAAH4IAQAwMAAAAQAgAKglAACmSgEAICAAAAEAIACoEAAATnABABAQAAABACAAaAQAAPaAAQAoAAAAgAAAAAABAAABACAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMzP/MzMz/zMzM/8zMz" (truncated))
   ├──1.52 MB (99.99%) ── malloc-heap/two-byte
   └──0.00 MB (00.01%) ── gc-heap/two-byte
Attached file webext-issue-1652925.zip (obsolete) —

This is a minimal extension with which to reproduce the favicon data URI leaks described in comment #24. The extension just call tabs.executeScript() for the current call to inject an empty content script.

  1. Load as temporary extension
  2. Navigate to http://www.pyrunner.com/weblog/2016/05/26/compressed-sensing-python/
  3. Click the extension button in the toolbar
  4. Repeat 3 multiple times.
  5. Close the tab http://www.pyrunner.com/weblog/2016/05/26/compressed-sensing-python/
  6. Repeat 2-5 multiple times just to be sure
  7. Navigate to about:memory
  8. Click GC, CC, Minimize memory as often as you wish
  9. Click Measure
  10. Filter output using x-icon

Result: multiple copies of large data URI of favicons .

Attachment #9187279 - Attachment is obsolete: true
Attached file webext-issue-1652925.zip (obsolete) —
Attachment #9187281 - Attachment is obsolete: true

Never mind comment #26, I can get multiple copies of data URI favicons without any extension, by just loading http://www.pyrunner.com/weblog/2016/05/26/compressed-sensing-python/ multiple times, then closing all tabs and measuring the memory as described above. The screenshot is what a user sent me.

Sorry, I was in a hurry yesterday night and the minimalist extension I attached was flawed. I took my time this morning and here is the proper minimalist extension to systematically reproduce the data URI leak, one guaranteed leak per click. The steps in comment #26 still apply, though there is no need for step 6.

My observations:

  • The number of copies of data URI string in the extension process is equal to the number of time executeScript() was called, i.e. the number of time the extension icon in the toolbar was clicked.
  • The data URI leak occurs only when the content script is executed through tabs.executeScript(), not when executed declaratively through manifest's content_scripts entry.
  • It's really the minimal number of code:
    • tabs.executeScript() in background page required
    • browser.runtime.onConnect.addListener() in background page is required
    • browser.runtime.connect() in content script is required
  • Note that nowhere are the Port object references kept around, so the leak can't be attributed the the minimalist extension

I am not sure this is conclusive, but I tried to attach a huge "trace" string (1M characters) to port.sender.tab.trace, where the favIconUrl property reside, and when I measure memory usage in about:memory, I can't find trace of that string -- i.e. I clicked the icon 10 times and though I can spot the favIconUrl strings in the report, I can't find the trace ones. Maybe this suggests that it's the favIconUrl property itself that is leaking, not the parent tab object?

As far as my limited understanding allows me, the favIconUrl is set at this place in Firefox's code:

https://searchfox.org/mozilla-central/rev/5a1a34953a26117f3be1a00db20c8bbdc03273d6/toolkit/components/extensions/parent/ext-tabs-base.js#679

For my extension, I found that I can set port.sender to undefined in my port connection handler to seemingly get rid of those favIconUrl leaks in the extension process, so for now I will do this until this is fixed in Firefox.

Thanks Raymond for the minimal reproduction case.

Based on the description, I suspect that something else is (indirectly) holding a reference to the Port instance as a whole.
After some code inspection, I suspect that the leak is caused by https://searchfox.org/mozilla-central/rev/5a1a34953a26117f3be1a00db20c8bbdc03273d6/toolkit/components/extensions/ExtensionCommon.jsm#506

That is not good... I'm going to fix it.

Assignee: nobody → rob
Status: NEW → ASSIGNED
Keywords: regression
Priority: P2 → P1
Regressed by: 1604058
Has Regression Range: --- → yes
Blocks: 1676930

Extension ports should be eligible for garbage collection when
disconnected. This did not happen because there was a strong reference
from the context to the conduit, whose subject was the Port. As a
result, the Port instances were not GCd until the context was unloaded.

This results in significant memory leaks over time, because it is not
uncommon for extensions to have a long-lived background page that
receives messages via Ports. The issue is made even worse by the fact
that ports contain metadata that can potentially be very large.

There are other callers of openConduit, but these are not affected
because their lifetimes are similar to the BaseContext.

Pushed by rob@robwu.nl:
https://hg.mozilla.org/integration/autoland/rev/fe39b455f8d9
Fix memory leak of extension Port via conduits r=zombie

https://hg.mozilla.org/mozilla-central/rev/fe39b455f8d9

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox85: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 85 Branch
See Also: → 1669626
Flags: in-testsuite+
Component: Developer Outreach → General

Comment on attachment 9187562 [details]
Bug 1652925 - Fix memory leak of extension Port via conduits

Beta/Release Uplift Approval Request

  • User impact if declined: Significant memory leaks in extensions, with multiple user reports in the order of gigabytes. Can result in OOM crashes of the extension process.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Small change with no difference in functional behavior except fixing the memory leak, fully covered by unit tests.
  • String changes made/needed:

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Causes significant memory leaks.
  • User impact if declined: Significant memory leaks in extensions, with multiple user reports in the order of gigabytes. Can result in OOM crashes of the extension process.
  • Fix Landed on Version: 85
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Small change with no difference in functional behavior except fixing the memory leak, fully covered by unit tests.
  • String or UUID changes made by this patch:
Attachment #9187562 - Flags: approval-mozilla-release?
Attachment #9187562 - Flags: approval-mozilla-esr78?
Attachment #9187562 - Flags: approval-mozilla-beta?
See Also: 1669626 →

Comment on attachment 9187562 [details]
Bug 1652925 - Fix memory leak of extension Port via conduits

Approved for 84.0b2.

Attachment #9187562 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment on attachment 9187562 [details]
Bug 1652925 - Fix memory leak of extension Port via conduits

approved for 78.6esr

Attachment #9187562 - Flags: approval-mozilla-esr78? → approval-mozilla-esr78+

eslint failure is because esr78 uses eslint 6.8.0, whereas m-c has since been updated to use 7.x, starting with 7.5.0 in bug 1620537. When I update eslint on esr78 to eslint@7.5.0, the linting error disappears.

I think that the appetite for updating eslint on esr78 is low, so alternatives are to quiet the warning, either by adding void at the front of the line, or by prepending // eslint-disable-next-line no-unused-expressions before the line.

So one of these:

void this.closeCallback?.();

or

// eslint-disable-next-line no-unused-expressions

I'll attach a patch with the latter.

Flags: needinfo?(rob)

ESR78 uses eslint 6.8.0, which triggers the no-unused-expressions
warning when optional chaining is used, despite the expression
opbviously not being unused (as a function call).

This issue does not occur on m-c because it uses a recent version of
eslint: eslint@7.5.0 and later support optional chaining without
external plugins
(https://eslint.org/blog/2020/07/eslint-v7.5.0-released#enhancements).

Instead of updating eslint@7.5.0 or newer on the ESR78 branch, this
patch adds a comment to ignore the false positive.

Thanks Rob!

Attachment #9187562 - Flags: approval-mozilla-release? → approval-mozilla-release-

Is this issue related/analogous? (Extension: duplicate tabs closer) - there are other extensions with similar strings

https://user-images.githubusercontent.com/35188272/102454716-b0c56600-3ffb-11eb-9d79-c66e5f162de9.PNG

https://github.com/Peuj/duplicate-tabs-closer/issues/50

(In reply to mkeroppi from comment #47)

Is this issue related/analogous? (Extension: duplicate tabs closer) - there are other extensions with similar strings

https://user-images.githubusercontent.com/35188272/102454716-b0c56600-3ffb-11eb-9d79-c66e5f162de9.PNG

https://github.com/Peuj/duplicate-tabs-closer/issues/50

This extension maintains an explicit map of tabs. That's is responsible for the large number of favicon strings in memory. The favicon URL seems to be used as part of the extension functionality. In other words, definitely not a bug in Firefox, and likely expected behavior in the extension as well.

(In reply to Rob Wu [:robwu] from comment #48)

(In reply to mkeroppi from comment #47)

Is this issue related/analogous? (Extension: duplicate tabs closer) - there are other extensions with similar strings

https://user-images.githubusercontent.com/35188272/102454716-b0c56600-3ffb-11eb-9d79-c66e5f162de9.PNG

https://github.com/Peuj/duplicate-tabs-closer/issues/50

This extension maintains an explicit map of tabs. That's is responsible for the large number of favicon strings in memory. The favicon URL seems to be used as part of the extension functionality. In other words, definitely not a bug in Firefox, and likely expected behavior in the extension as well.

The number of strings seems excessive to me (given the low number of tabs opened). Video DownloadHelper has a similar number of strings.

(In reply to mkeroppi from comment #49)

(In reply to Rob Wu [:robwu] from comment #48)

(In reply to mkeroppi from comment #47)

Is this issue related/analogous? (Extension: duplicate tabs closer) - there are other extensions with similar strings

https://user-images.githubusercontent.com/35188272/102454716-b0c56600-3ffb-11eb-9d79-c66e5f162de9.PNG

https://github.com/Peuj/duplicate-tabs-closer/issues/50

This extension maintains an explicit map of tabs. That's is responsible for the large number of favicon strings in memory. The favicon URL seems to be used as part of the extension functionality. In other words, definitely not a bug in Firefox, and likely expected behavior in the extension as well.

The number of strings seems excessive to me (given the low number of tabs opened). Video DownloadHelper has a similar number of strings.

Seems to garbage collect correctly after some time.

(In reply to mkeroppi from comment #50)

(In reply to mkeroppi from comment #49)

(In reply to Rob Wu [:robwu] from comment #48)

(In reply to mkeroppi from comment #47)

Is this issue related/analogous? (Extension: duplicate tabs closer) - there are other extensions with similar strings

https://user-images.githubusercontent.com/35188272/102454716-b0c56600-3ffb-11eb-9d79-c66e5f162de9.PNG

https://github.com/Peuj/duplicate-tabs-closer/issues/50

This extension maintains an explicit map of tabs. That's is responsible for the large number of favicon strings in memory. The favicon URL seems to be used as part of the extension functionality. In other words, definitely not a bug in Firefox, and likely expected behavior in the extension as well.

The number of strings seems excessive to me (given the low number of tabs opened). Video DownloadHelper has a similar number of strings.

Seems to garbage collect correctly after some time.

Just for information:
The map of tabs doesn't store the favicon strings, it only store the url, a date and a boolean.
The favicon referenced here https://github.com/Peuj/duplicate-tabs-closer/blob/085a9db71cb86518949457a75c8ae6dc006fde0c/worker.js#L255 is retrieved for duplicate tabs (which are most of the case few) and sent to the opened popup panel.
So there is probably a memory issue but I don't think it's related with your points.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: