1379250 - Memory leak when setting PAC script

Status: RESOLVED DUPLICATE of bug 1377738

(Core :: Networking: HTTP, defect)

Description

[Alexander Grimalovsky]

Attachment: 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.

Comment 1

[Alexander Grimalovsky]

Attachment: 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.

Comment 2

[bekervincent]

<!--put button  on bug-->

Comment 3

[Alexander Grimalovsky]

(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?

Comment 4

[Alexander Grimalovsky]

Removing "needinfo" from me until there will be some real request for information

Comment 5

[Gary Chen [:xeonchen]]

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]

Comment 6

[Gary Chen [:xeonchen]]

Hi Alexander, I can reproduce this on beta, but not on nightly. Can you confirm that?

Comment 7

[Gary Chen [:xeonchen]]

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?

Comment 8

[Alexander Grimalovsky]

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.

Comment 9

[Gary Chen [:xeonchen]]

(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.

Comment 10

[Alexander Grimalovsky]

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)

Comment 11

[Jan de Mooij [:jandem]]

(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?

Comment 12

[Jan de Mooij [:jandem]]

Jon, if you agree with comment 11, maybe we could backport bug 1377738 if it helps certain add-on users.

Comment 13

[Jon Coppeard (:jonco)]

It sounds like that's the problem.  Let's see if we can backport the fix.

Comment 14

[Gary Chen [:xeonchen]]

(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.