Memory leak when setting PAC script

RESOLVED DUPLICATE of bug 1377738

Status

()

Core
Networking: HTTP
RESOLVED DUPLICATE of bug 1377738
5 months ago
4 months ago

People

(Reporter: Alexander Grimalovsky, Assigned: xeonchen)

Tracking

55 Branch
x86
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [necko-active][proxy][MemShrink])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 months ago
Created attachment 8884352 [details]
test-pac-memory-leak.xpi

Setting PAC script through network.proxy.autoconfig_url setting seems to cause memory leak when network.proxy.type is set to 2 (auto-configuration by url).

I've prepared small test add-on to demonstrate the problem. Its whole purpose is to perform 1000 iterations of setting network.proxy.autoconfig_url like this:

preferences.set('network.proxy.autoconfig_url', `data:text/plain, function FindProxyForURL(url, host) /* ${Math.random()} */ { return 'DIRECT';} }`);

Math.random() is used to generate different PAC scripts on each iteration.

Running provided test add-on gives following results on freshly installed Firefox 54.0.1 x86 with new profile on Windows 7 x64:

before:
memory: 252mb
swap: 250mb

after:
memory: 298mb
swap: 1257mb

As can be seen from these numbers - simple repetitive setting of trivial PAC script results into additional 1Gb of swap memory to be consumed.

When PAC script use is not enabled (network.proxy.type != 2) or when same PAC script is set multiple times (no Math.random() in line above) - memory usage stays more or less same.

This issue is critical for add-ons that works with PAC scripts, e.g. various proxy services.
(Reporter)

Comment 1

5 months ago
Created attachment 8884361 [details]
Fixed version of add-on that produces valid PAC script

I've just found that PAC script code contains additional closing curly brace that cases resulted JavaScript code to be invalid. Please find attached updated version of add-on that have no such issue. However it doesn't affect the problem itself.

One more information that I've missed into initial report: complexity of PAC script itself doesn't seem to affect issue. I've tried to generate and set PAC script with 50+ rules inside but resulted amount of memory is roughly same.
Attachment #8884352 - Attachment is obsolete: true

Comment 2

4 months ago
<!--put button  on bug-->
Flags: needinfo?(alexander.grimalovsky)
(Reporter)

Comment 3

4 months ago
(In reply to bekervincent from comment #2)
> <!--put button  on bug-->

Sorry, I didn't understand your comment. What information do you need from me?
(Reporter)

Comment 4

4 months ago
Removing "needinfo" from me until there will be some real request for information
Flags: needinfo?(alexander.grimalovsky)
(Assignee)

Updated

4 months ago
Assignee: nobody → xeonchen
Whiteboard: [necko-active][proxy]
(Assignee)

Comment 5

4 months ago
I can see some potential leak due to posting an event while the thread is shutting down.

* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  * frame #0: 0x0000000102c701d8 XUL`nsThread::PutEvent(this=0x000000018cb32600, aEvent=<unavailable>, aTarget=0x0000000000000000) at nsThread.cpp:735 [opt]
    frame #1: 0x0000000102c7053b XUL`nsThread::DispatchInternal(this=0x000000018cb32600, aEvent=<unavailable>, aFlags=<unavailable>, aTarget=0x0000000000000000) at nsThread.cpp:809 [opt]
    frame #2: 0x0000000102c70943 XUL`nsThread::Dispatch(this=0x000000018cb32600, aEvent=(mRawPtr = 0x0000000000000000), aFlags=0) at nsThread.cpp:868 [opt]
    frame #3: 0x0000000102c2c622 XUL`nsIEventTarget::Dispatch(this=0x000000018cb32600, aEvent=<unavailable>, aFlags=0) at nsIEventTarget.h:37 [opt]
    frame #4: 0x0000000102d4b4db XUL`mozilla::net::nsPACMan::PostProcessPendingQ(this=0x000000018da4ee40) at nsPACMan.cpp:556 [opt]
    frame #5: 0x0000000102d55959 XUL`mozilla::net::PACLoadComplete::Run(this=0x00000001832b2790) at nsPACMan.cpp:186 [opt]
    frame #6: 0x0000000102c7215b XUL`nsd::ProcessNextEvent(this=<unavailable>, aMayWait=<unavailable>, aResult=0x00007fff5fbfd467) at nsThread.cpp:1418 [opt]
    frame #7: 0x0000000102c77e75 XUL`NS_ProcessNextEvent(aThread=0x000000010d440800, aMayWait=true) at nsThreadUtils.cpp:475 [opt]
    frame #8: 0x0000000102c714a5 XUL`nsThread::Shutdown() [inlined] bool mozilla::SpinEventLoopUntil<(mozilla::ProcessFailureBehavior)1, nsThread::Shutdown()::$_2>(nsThread::Shutdown()::$_2&&, nsIThread*) at nsThreadUtils.h:281 [opt]
    frame #9: 0x0000000102c7146f XUL`nsThread::Shutdown(this=0x000000018cb32600) at nsThread.cpp:1032 [opt]
    frame #10: 0x0000000102d553ea XUL`mozilla::net::WaitForThreadShutdown::Run(this=0x000000011d844400) at nsPACMan.cpp:158 [opt]
    frame #11: 0x0000000102c7215b XUL`nsThread::ProcessNextEvent(this=<unavailable>, aMayWait=<unavailable>, aResult=0x00007fff5fbfd5b7) at nsThread.cpp:1418 [opt]
    frame #12: 0x0000000102c6f503 XUL`NS_ProcessPendingEvents(aThread=0x000000010d440800, aTimeout=10) at nsThreadUtils.cpp:417 [opt]
    frame #13: 0x0000000105603f31 XUL`nsBaseAppShell::NativeEventCallback(this=0x000000010d4873a0) at nsBaseAppShell.cpp:97 [opt]
    frame #14: 0x000000010566cbb7 XUL`nsAppShell::ProcessGeckoEvents(aInfo=0x000000010d4873a0) at nsAppShell.mm:399 [opt]
Whiteboard: [necko-active][proxy] → [necko-active][proxy][MemShrink]
(Assignee)

Comment 6

4 months ago
Hi Alexander, I can reproduce this on beta, but not on nightly. Can you confirm that?
Flags: needinfo?(alexander.grimalovsky)
(Assignee)

Comment 7

4 months ago
I added some log to mmap/munmap/MozTaggedAnonymousMmap in js/src/gc/Memory.cpp, and found that every 2 memory allocation matches only 1 release.

[MozTaggedAnonymousMmap] 0x1a6300000 (1048576)
[MozTaggedAnonymousMmap] 0x1a6500000 (1048576)
[munmap] 0x1a6400000 (1048576)
[MozTaggedAnonymousMmap] 0x1a6400000 (1048576)
[MozTaggedAnonymousMmap] 0x1a6600000 (1048576)
[munmap] 0x1a6500000 (1048576)
[MozTaggedAnonymousMmap] 0x1a6500000 (1048576)
[MozTaggedAnonymousMmap] 0x1a6700000 (1048576)
[munmap] 0x1a6600000 (1048576)

where the addresses that NOT being released are acquired by |gc.getOrAllocChunk| within |js::Nursery::updateNumChunksLocked|.

Jan, do you have any idea?
Flags: needinfo?(jdemooij)
(Reporter)

Comment 8

4 months ago
I can confirm problem on latest build from beta channel.

Latest nightly build at a time of writing this comment seems to be seriously broken, so I was unable to test with it.

I've tried to test on this build: http://ftp.mozilla.org/pub/firefox/nightly/2017/07/2017-07-11-03-02-03-mozilla-central/
but was unable to install add-on due to "incompatible version" error from Firefox upon attempt to install add-on through "about:debugging#addons" screen. I was unable to solve this problem and hence can't provide you with updated information.

Gary, can you please provide me with a tip on how did you manage to install test add-on on nightly build? I will be able to test it then.
Flags: needinfo?(xeonchen)
(Assignee)

Comment 9

4 months ago
(In reply to Alexander Grimalovsky from comment #8)
> I can confirm problem on latest build from beta channel.
> 
> Latest nightly build at a time of writing this comment seems to be seriously
> broken, so I was unable to test with it.
> 
> I've tried to test on this build:
> http://ftp.mozilla.org/pub/firefox/nightly/2017/07/2017-07-11-03-02-03-
> mozilla-central/
> but was unable to install add-on due to "incompatible version" error from
> Firefox upon attempt to install add-on through "about:debugging#addons"
> screen. I was unable to solve this problem and hence can't provide you with
> updated information.
> 
> Gary, can you please provide me with a tip on how did you manage to install
> test add-on on nightly build? I will be able to test it then.

Sorry I didn't aware that it needs some tweaks to install your add-on on Nightly.
Please follow these steps:

- enter about:config and set "extensions.allow-non-mpc-extensions" to true.
- enter about:debugging#addons and press "Load Temporary Add-on", then select your add-on file.

After my installation, I can see "network.proxy.autoconfig_url" keep changing. I assume that means the add-on is working.
Flags: needinfo?(xeonchen)
(Reporter)

Comment 10

4 months ago
Gary, I can confirm that issue is not reproducible on latest nightly build that I was able to run properly: 56.0a1 (2017-07-11) (32-bit)
Flags: needinfo?(alexander.grimalovsky)
(In reply to Gary Chen [:xeonchen] (needinfo plz) from comment #7)
> where the addresses that NOT being released are acquired by
> |gc.getOrAllocChunk| within |js::Nursery::updateNumChunksLocked|.
> 
> Jan, do you have any idea?

It's possible this was fixed by bug 1377738. Maybe someone can confirm that so we can dupe this?
Flags: needinfo?(jdemooij)
Jon, if you agree with comment 11, maybe we could backport bug 1377738 if it helps certain add-on users.
Flags: needinfo?(jcoppeard)
It sounds like that's the problem.  Let's see if we can backport the fix.
Flags: needinfo?(jcoppeard)
(Assignee)

Comment 14

4 months ago
(In reply to Jan de Mooij [:jandem] from comment #11)
> (In reply to Gary Chen [:xeonchen] (needinfo plz) from comment #7)
> > where the addresses that NOT being released are acquired by
> > |gc.getOrAllocChunk| within |js::Nursery::updateNumChunksLocked|.
> > 
> > Jan, do you have any idea?
> 
> It's possible this was fixed by bug 1377738. Maybe someone can confirm that
> so we can dupe this?

After applying the patch of bug 1377738, the leakage issue is gone.
Thank you Jan, I think we can close this as a dup.
Status: NEW → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1377738
You need to log in before you can comment on or make changes to this bug.