738640 - [10.7] crash in -[BaseWindow dealloc], caused by bug in RoboForm extension

Status: RESOLVED WORKSFORME

(Core :: Widget: Cocoa, defect)

Description

[Scoobidiver (away)]

It's #8 top crasher in 12.0b1 on Mac OS X.
It first appeared in 12.0a2/20120206 but is discontinuous across builds.

Signature 	AppKit@0xd7851 More Reports Search
UUID	5f6b4db5-606b-4649-9466-07ed12120323
Date Processed	2012-03-23 12:02:06
Uptime	4074
Last Crash	1.5 days before submission
Install Age	10.6 hours since version was first installed.
Install Time	2012-03-23 01:09:33
Product	Firefox
Version	12.0
Build ID	20120321033733
Release Channel	beta
OS	Mac OS X
OS Version	10.7.3 11D50
Build Architecture	amd64
Build Architecture Info	family 6 model 37 stepping 2
Crash Reason	EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Crash Address	0x9d02867
App Notes 	
AdapterVendorID: 0x10de, AdapterDeviceID: 0x a29GL Context? GL Context+ GL Layers? GL Layers+ 
EMCheckCompatibility	True

Frame 	Module 	Signature 	Source
0 		@0x109d02867 	
1 	AppKit 	AppKit@0xd7851 	
2 	AppKit 	AppKit@0xd0abd 	
3 	AppKit 	AppKit@0xd080c 	
4 	libmozglue.dylib 	je_calloc 	_string.h:80
5 	libobjc.A.dylib 	libobjc.A.dylib@0x5b0a 	
6 	libobjc.A.dylib 	libobjc.A.dylib@0x5fe2 	
7 	libobjc.A.dylib 	libobjc.A.dylib@0x5b0a 	
8 	libobjc.A.dylib 	libobjc.A.dylib@0xf507 	
9 	libobjc.A.dylib 	libobjc.A.dylib@0xbe79 	
10 	CoreFoundation 	CoreFoundation@0x71357 	
11 	AppKit 	AppKit@0x11c19a 	
12 	libsystem_c.dylib 	libsystem_c.dylib@0x4d46f 	
13 	libsystem_c.dylib 	libsystem_c.dylib@0x4d46f 	
14 	libsystem_c.dylib 	libsystem_c.dylib@0x4d6aa 	
15 	CoreFoundation 	CoreFoundation@0x93a83 	
16 	AppKit 	AppKit@0x472e0 	
17 	AppKit 	AppKit@0x4d86f 	
18 	AppKit 	AppKit@0x241b56 	
19 	XUL 	-[BaseWindow dealloc] 	widget/cocoa/nsCocoaWindow.mm:2073
20 	AppKit 	AppKit@0x42144 	
21 	libobjc.A.dylib 	libobjc.A.dylib@0xed52 	
22 	CoreFoundation 	CoreFoundation@0x31275 	
23 	libobjc.A.dylib 	libobjc.A.dylib@0xf03b 	
24 	CoreFoundation 	CoreFoundation@0x16d73b 	
25 	CoreFoundation 	CoreFoundation@0x389a0 	
26 	CoreFoundation 	CoreFoundation@0xf7e2 	
27 	libsystem_c.dylib 	libsystem_c.dylib@0x4e371 	
28 	libsystem_c.dylib 	libsystem_c.dylib@0x6a710 	
29 	libsystem_c.dylib 	libsystem_c.dylib@0x4d46f 	
30 	libsystem_c.dylib 	libsystem_c.dylib@0x4d6aa 	
31 	AppKit 	AppKit@0x982c8a 	
32 	AppKit 	AppKit@0x982c8a 	
33 	libobjc.A.dylib 	libobjc.A.dylib@0xcd08 	
34 	AppKit 	AppKit@0x982c8a 	
35 	libobjc.A.dylib 	libobjc.A.dylib@0xce75 	
36 	libobjc.A.dylib 	libobjc.A.dylib@0xc417 	
37 	libsystem_c.dylib 	libsystem_c.dylib@0x4d46f 	
38 	libsystem_c.dylib 	libsystem_c.dylib@0x4d6aa 	
39 	CoreFoundation 	CoreFoundation@0x182cb3 	
40 	CoreFoundation 	CoreFoundation@0x182cb3 	
41 	libmozglue.dylib 	je_free 	memory/jemalloc/jemalloc.c:1662
42 	XUL 	nsVoidArray::operator= 	obj-firefox/x86_64/xpcom/build/nsVoidArray.cpp:366
43 		@0x100200043 	
44 	libmozglue.dylib 	je_free 	memory/jemalloc/jemalloc.c:1662
45 		@0x7fff5fbff17f 	
46 	CoreFoundation 	CoreFoundation@0x31b04 	
47 	AppKit 	AppKit@0x888adf 	
48 	Foundation 	Foundation@0x951c 	
49 	XUL 	ScopedXPCOMStartup::~ScopedXPCOMStartup 	toolkit/xre/nsAppRunner.cpp:1113
50 	XUL 	XRE_main 	toolkit/xre/nsAppRunner.cpp:1115
51 	firefox 	main 	browser/app/nsBrowserApp.cpp:205
52 	firefox 	firefox@0x1433 	

More reports at:
https://crash-stats.mozilla.com/report/list?signature=AppKit%400xd7851

Comment 1

[Scoobidiver (away)]

More reports with a better stack also at:
https://crash-stats.mozilla.com/report/list?signature=libobjc.A.dylib%400xb350

Comment 2

[Marcia Knous [:marcia]]

I have been able to reproduce this crash once, albeit with a slight different stack using 12.0b2:

https://crash-stats.mozilla.com/report/index/bp-ae3793c9-15ca-4d8d-b17c-a51ba2120323

STR:

1. Go to https://cloudmagic.com and create an account. (Note: You may be able to skip this and just try https://addons.mozilla.org/en-US/firefox/addon/cloudmagic/eula/147475?src=dp-btn-primary but I did not follow that path)
2. After account has been established, click the button to install the Firefox extension (it will be at the top of the page)
3. I forced the extension to install even though it was not compatible.
4. Restart Firefox

I crashed after restarting. I had some other extensions installed as well, but cloudmagic was found among the extensions in the crash reports.

I will see if I can reproduce after lunch with a fresh profile and if so will add the keyword.

Comment 3

[Marcia Knous [:marcia]]

It seems the key to reproducing this bug actually involves more than one extension - here is my extension set:

CloudMagic - Exchange, Gmail and Twitter Search
1.1.6
true
cloudmagic@cloudmagic
RoboForm
"1-00-002"
true
[e-mail]
Test Pilot
1.2.1
true
[e-mail]

So in addition to install Cloudmagic as described above, you should also:

1. Install http://www.roboform.com/platforms/browsers/safari. 
2. Once the browser restarts, allow the installation of the Roboform extension which is designated as "1-00-002"
3. At this point you can then install cloud magic, or uninstall if it is already installed.
4. After installing or uninstalling the cloud magic addon, I get this crash - https://crash-stats.mozilla.com/report/index/bp-92f31cc6-9607-4744-b185-6c80c2120323

Comment 4

[Steven Michaud [:smichaud] (Retired)]

I'm unable to reproduce	Marcia's STR.  I tested with today's
mozilla-central nightly on OS X 10.7.3.  Here's what I did:

1) Once I'd downloaded roboform-mac.dmg	from
   http://www.roboform.com/platforms/browsers/safari to my Desktop, I
   copied the RoboForm app to my Desktop and ran it.

   I didn't create an account or activate RoboForm.

2) Started today's mozilla-central nightly.

   I was prompted to install the RoboForm "1-00-02" extension, which I
   allowed.  Then I clicked the "Restart Nightly" button.

3) Installed the CloudMagic extension without creating an account, by
   visiting
   https://addons.mozilla.org/en-US/firefox/addon/cloudmagic/eula/147475?src=dp-btn-primary
   and clicking	the "Restart Now" button.

My other extensions are	Test Pilot 1.2.1 and PDF Viewer	0.2.414.  I've
left the PDF Viewer disabled.

Marcia:

Did you restart right after installing the RoboForm extension?

Can you consistently reproduce the crash just by installing or
uninstalling the CloudMagic extension, then restarting?  (If you have
the other extensions installed that you listed.)

Comment 5

[Steven Michaud [:smichaud] (Retired)]

RoboForm is a pain to uninstall completely.  Here are the
files/directories created by RoboForm that I've been able to find,
which I've been deleting:

~/.goodsync
~/.roboform
~/Documents/Logins/Contact Info.rfp
~/Documents/Logins/cache.rfo
~/Documents/Logins/mru.rfo
~/Documents/Logins/options.rfo
~/Library/Application Support/RoboForm
~/Library/Caches/com.SilberSystems.RoboForm
~/Library/Internet Plug-Ins/rf-firefox-plugin.plugin
~/Library/Internet Plug-Ins/rf-safari-plugin.webplugin
~/Library/Preferences/com.SiberSystems.RoboForm.plist
~/Library/Preferences/com.SiberSystems.RoboForm.plist.lockfile
~/Library/Preferences/com.SiberSystems.RoboFormIcon.plist
~/Library/Preferences/com.SiberSystems.RoboFormIcon.plist.lockfile
~/Library/Saved	Application State/com.SiberSystems.RoboForm.savedState

Comment 6

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #5)

Another thing about the RoboForm app -- it seems to be impossible to quit :-(

So besides deleting the above-mentioned files and directories, you also need to log out or restart.

Comment 7

[Steven Michaud [:smichaud] (Retired)]

Interestingly, I *can* reproduce with FF 12.0b2 (the latest beta), though I can't with today's Aurora nightly.

I suspect this has something to do with the FF release branding.  I'll do a custom trunk build with the release branding, and see what happens.

Comment 8

[Marcia Knous [:marcia]]

Steven: I have only tested this with 12.0b2, and not with nightly. And with that build I can reproduce consistently either installing or uninstalling Cloud Magic.

(In reply to Steven Michaud from comment #7)
> Interestingly, I *can* reproduce with FF 12.0b2 (the latest beta), though I
> can't with today's Aurora nightly.
> 
> I suspect this has something to do with the FF release branding.  I'll do a
> custom trunk build with the release branding, and see what happens.

Comment 9

[Steven Michaud [:smichaud] (Retired)]

> I'll do a custom trunk build with the release branding, and see what
> happens.

This wasn't enough to trigger the crashes.  Neither was forcing the
version number to "12.0b2".

So this bug may be "fixed" in current code.

Next I'll try reproducing it in code corresponding exactly to the
(real) 12.0b2 release.

Comment 10

[Steven Michaud [:smichaud] (Retired)]

Attachment: Gdb stack trace

Here's a gdb stack trace of this crash, obtained using a custom (non-opt, non-debug) build made from current beta-branch code.

If I force the version number to "12.0", I can reproduce these crashes on the beta branch as far back as I've looked (to the 10 branch).  So (for the moment at least) I've given up trying to find a regression range.

Despite all the tricks I've played, I haven't been able to reproduce these crashes on the trunk or the aurora branch.  But I suspect this is just an accident -- that the bug (whatever it is) also effects those branches.

Comment 11

[Steven Michaud [:smichaud] (Retired)]

This is probably related to bug 720390 and bug 723201.

Comment 12

[Sheila Mooney]

Steven, do we have any ideas about a fix here? The combined signature gives us about 50 crashes a week on Mac which puts it in the top 5.

Comment 13

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #12)

I've been working on it for the past few days, and have made some progress -- for example I've discovered that this is at least partly a RoboForm bug.  But I haven't yet finished my analysis.

What's good is that I can reproduce the bug consistently, and it appears to be in Objective-C code (which is relatively easy to debug).  So it may take me another few days, but I'm reasonably certain I'll be able to find a fix/workaround for the bug (or bugs) here.

Comment 14

[Steven Michaud [:smichaud] (Retired)]

I'm close to posting a patch for this, but I'm still not finished.

This is a RoboForm bug.

The RoboForm extension uses JavaScript code to load its plugin into
the main process.  The extension calls the plugin to add some Cocoa
objects to the window and view hierarchy, and swizzles some
Objective-C methods.  Then, as the browser quits, the extension
unloads the plugin (via a call from JavaScript (indirectly) to
PR_UnloadLibrary()).  *But the plugin doesn't first undo the changes
it made.*  So we're left with a bunch of dangling pointers :-(

I've already found and worked around two kinds of crashes -- one
caused by a dangling pointer to	a hook for the -[NSWindow isKeyWindow]
method,	and the	other caused by	dangling pointers to a custom subclass
of NSWindow (RFChildWindow).  But (after removing a bunch of
debug-logging code) I've apparently now bumped up against a third kind
(which I suspect involves one or more custom subclasses of NSView).
So I'll be a little longer.

But I do hope to post a complete patch tomorrow.

Comment 15

[Steven Michaud [:smichaud] (Retired)]

Attachment: Patch (work around dangling pointers)

Here's a patch that fixes these crashes in my tests.

Turns out I'd made a mistake translating from my debugging patch to
the final patch.  So no, I didn't encounter a third kind of crash that
also needs fixing (it was one of the two I'd previously discovered).

To recap, when the RoboForm extension unloads the RoboForm plugin (as
the browser quits), it leaves a bunch of dangling pointers.  These
cause two different kinds of crashes, which my patch works around:

1) A crash accessing the IMP object for one of the methods that the
   RoboForm plugin swizzles (-[NSWindow isKeyWindow]).  The stack in
   attachment 610165 [details], though incomplete at the top, represents this
   kind of crash.

   I work around this (as the browser quits) by unswizzling all the
   methods that the RoboForm plugin has swizzled.  (More in a later
   comment about how I found out which methods these are.)

   I might have	gotten away with only unswizzling the -[NSWindow
   isKeyWindow] method.  But it's probably safer to do them all, and
   this	isn't difficult.

2) A crash accessing the Class object for one of the custom classes
   created by the RoboForm plugin -- RFChildWindow.

   I work around this (as the browser quits) by	detaching any
   RFChildWindow objects still in NSApplication's window list.

   The RoboForm plugin also creates many other kinds of custom
   classes, and instances thereof.  But it only creates one
   RFChildWindow, which it makes a child of one of the browser
   windows.  And (as best I can tell) it doesn't attach any of its
   other custom objects directly to a non-RoboForm native object.

   If I'm right about this our problem is solved.  But if I'm wrong
   we'll keep seeing crashes on quit that are 100% correlated with the
   RoboForm extension.  We'll need to keep our eyes open for this.

   Note that the RoboForm plugin (rf-firefox-plugin) *won't* appear in
   the Breakpad list of modules, since it's been unloaded by the time
   any of these crashes happen.

I'm attempting to do a beta-branch tryserver build of my patch.  If
that works out, it should be available in a few hours.

Comment 16

[Steven Michaud [:smichaud] (Retired)]

Here's how I found out which methods the RoboForm plugin swizzles:

First I ran class-dump and "nm -pam" on the plugin (rf-firefox-plugin
in ~/Library/Internet Plug-Ins/).  class-dump gives you header
information on all the plugin's	custom Objective-C classes.  "nm -pam"
gives you all the plugin's symbols (exported or not).

One of the symbols is:

__Z7swizzleP10objc_classP13objc_selectorS1_

c++filt translates this as follows:

swizzle(objc_class*, objc_selector*, objc_selector)

You can break on this in gdb.  The first parameter is the Class
object, one of whose methods you want to swizzle.  The second is the
selector (SEL object) for the method you want to swizzle.  The third
is the selector for the method you want to replace it with.

Both the second and the third parameters can be treated as SEL objects
(as pointers to objc_selector) -- for example you can call
NSStringFromSelector() on both of them.

Comment 18

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #15)

>   The RoboForm plugin also creates many other kinds of custom
>   classes, and instances thereof.  But it only creates one
>   RFChildWindow, which it makes a child of one of the browser
>   windows.  And (as best I can tell) it doesn't attach any of its
>   other custom objects directly to a non-RoboForm native object.

Oops, this is partially incorrect.  The RoboForm plugin creates one RFChildWindow object for every browser window (every ToolbarWindow object), and attaches it to the browser window as a child window (using -[NSWindow addChildWindow:]).

Comment 19

[Steven Michaud [:smichaud] (Retired)]

I just realized there's a much simpler way to fix this bug -- just do
an extra call to dlopen() on the plugin!

PR_UnloadLibrary() just	calls dlclose().  But dlclose() uses reference
counting (it only unloads a library when its reference count has
reached zero).	So an extra call to dlopen() will increase the
reference count, and prevent it from reaching zero.

It makes no sense for the RoboForm extension to unload its plugin as
the browser quits -- the plugin will get unloaded very soon anyway.

New patch coming up.

Comment 20

[Steven Michaud [:smichaud] (Retired)]

Attachment: Alternate patch (prevent RoboForm plugin unloading)

Here's my new patch, which in my tests also fixes this bug's crashes.

It's simpler than my previous patch, and (because it gets at the root
cause) is guaranteed to "fix" all the dangling pointers (by preventing
them from dangling in the first place).

But along the way I started to wonder if there might be a good reason
for the RoboForm extension to explicitly unload its plugin.

As Firefox quits, it explicitly unloads a number of security-related
modules, presumably to prevent their contents from being available in
RAM after Firefox has quit.  However (as I understand it) just
unloading the modules doesn't do the trick -- you also have to clear
at least part of the RAM that they were using (the parts containing
sensitive data).

RoboForm stores user passwords, so some of the RAM it uses will
contain sensitive data.  But as best I can tell the RoboForm extension
doesn't do anything with the RAM freed after unloading its plugin.

But it's hard to be sure:  The JavaScript code from which the RoboForm
extension explicitly unloads its plugin is obfuscated.  And I don't
know what tools (if any) the browser makes available, callable from
JavaScript code, that the RoboForm extension might use to clear the
plugin's RAM after it's been freed.

I don't	know this code at all, and don't know who to ask about it.
Help! :-)

Comment 21

[Alex Keybl [:akeybl]]

Including dmandelin to see if he can help out with the JS questions above. Have we figured out the regressing bug in FF12 to consider backing out ahead of our next beta?

Comment 22

[Steven Michaud [:smichaud] (Retired)]

> Including dmandelin to see if he can help out with the JS questions
> above.

Thanks.

> Have we figured out the regressing bug in FF12 to consider backing
> out ahead of our next beta?

There isn't one.  This is entirely a RoboForm bug.

I still	don't know why it's only reproducible on the beta branch, with
the FF version forced to 12.  But it's possible that FF's versioning
changes RoboForm's behavior.  And I suspect the problem only started
with the most recent version of the RoboForm extension (0-9-90,
released on 2011-12-29).

Comment 23

[Steven Michaud [:smichaud] (Retired)]

Here's a tryserver build made with my original patch (attachment 612193 [details] [diff] [review] from comment #15):

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-030afb750fdb/try-macosx64/firefox-12.0.en-US.mac.dmg

In some ways it's not as good as my second patch (attachment 612365 [details] [diff] [review] from comment #20).  But we may end up having to fall back to it.  So please test it out!

Comment 24

[Steven Michaud [:smichaud] (Retired)]

Note that the tryserver build linked in comment #23 is a beta-branch build.

I've started another one for my alternate patch, from comment #20.

Comment 25

[Steven Michaud [:smichaud] (Retired)]

RoboForm does have a (well-hidden) way to report bugs (http://www.roboform.com/support/preticket_faq, bottom of the page).  But it'd be nice to find someone from RoboForm we can CC on this bug.

Can anyone help with this?  Thanks in advance!

Comment 26

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #20)

> I don't know what tools (if any) the browser makes available,
> callable from JavaScript code, that the RoboForm extension might use
> to clear the plugin's RAM after it's been freed.

I doubt	there's	any way	to wipe	the plugin's RAM *after* it's been
unloaded.  So here's a better way to ask this question:

What tools (if any) does the browser make available, callable from
JavaScript code, to ensure that the RAM used by a module will get
wiped by the time the module is unloaded?  (Explicitly unloaded from
JavaScript code, indirectly using a call to PR_UnloadLibrary().)

Comment 27

[Steven Michaud [:smichaud] (Retired)]

Here's a tryserver build (beta-branch) made from my alternate patch (attachment 612365 [details] [diff] [review]) from comment #20:

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-4c33da7901b7/try-macosx64/firefox-12.0.en-US.mac.dmg

Comment 28

[Markus Stange [:mstange]]

As far as I know, the recommended way to deal with crashes caused by extensions is to report the problem to the extension author, and then optionally blacklist unfixed versions of the extension either after a fixed version has been released or after a grace period has run out.

We really shouldn't need to add workarounds for specific extension to our code.

Comment 29

[Markus Stange [:mstange]]

CC'ing two Roboform contacts (found in bug 698018).

@Vadim: Description of the Roboform bug is in comment 14 and comment 15, suggested fix is to not unload the library on shutdown.

Comment 30

[Steven Michaud [:smichaud] (Retired)]

(Comment #28)

Yes, of course.  But I know that we often *do* include workarounds for OS bugs and plugin bugs.  The reason (of course) is that it usually takes them a lot longer to fix the problem than it does for us to work around it.

Comment 31

[Steven Michaud [:smichaud] (Retired)]

And this bug is a lot more serious than bug 698018 -- it's a topcrasher.  We can't afford to wait a long time to get it fixed.  In fact we can't afford to let the effects of this bug get into a release (FF 12).

Comment 32

[Steven Michaud [:smichaud] (Retired)]

> @Vadim: Description of the Roboform bug is in comment 14 and comment 15

Also see comment 19 and comment 20.

Comment 33

[Steven Michaud [:smichaud] (Retired)]

To the RoboForm people:

I particularly want to know if my "alternate patch" (attachment 612365 [details] [diff] [review], comment #20) might open a security hole in RoboForm.  Or to put it another way, will it be OK (securitywise) to stop the rf-firefox-plugin module from being unloaded until all the other Firefox modules are unloaded (by the OS, as the Firefox process dies).

Comment 34

[Steven Michaud [:smichaud] (Retired)]

The version of RoboForm that I've been testing with is 1.0.2.  But the current version is 1.0.7, so I've now started testing with that version, too.

It still has the bug, and the symptoms are exactly the same.

I've also found out that the bug doesn't happen (and RoboForm extension doesn't load its binary plugin) if you disable the RoboForm extension.

Comment 35

[Vadim Maslov]

so this is RoboForm for Mac, not RoboForm for Windows.

will send to developers.

Comment 36

[Steven Michaud [:smichaud] (Retired)]

> so this is RoboForm for Mac, not RoboForm for Windows.

Correct.

> will send to developers.

Thanks!

Comment 37

[Alex Keybl [:akeybl]]

(In reply to Steven Michaud from comment #22)
> There isn't one.  This is entirely a RoboForm bug.
> 
> I still	don't know why it's only reproducible on the beta branch, with
> the FF version forced to 12.  But it's possible that FF's versioning
> changes RoboForm's behavior.  And I suspect the problem only started
> with the most recent version of the RoboForm extension (0-9-90,
> released on 2011-12-29).

Does the issue reproduce if the Roboform extension is disabled? If not, then we have a Plan B of blocklisting affected Roboform versions.

Comment 38

[Steven Michaud [:smichaud] (Retired)]

> Does the issue reproduce if the Roboform extension is disabled?

No (see comment #34).

> If not, then we have a Plan B of blocklisting affected Roboform versions.

Correct.

Comment 39

[Vlad Marchenko]

so. I easily reproduce the problem on ff12b4. I will came up with the solution on monday. However nightly build work fine.

Comment 40

[Vlad Marchenko]

we did investigation and find out:
1. In ff12 during window initialization you recreate roboform toolbar view (NSView) twice. In nightly build you do not do this. 
2. This view double creation did broke some of our logic and this crash is just consequences of double creation. So there is no sense to fix this crash directly. 
3. So actually you can create and recreate NSView in toolbar as many as you want so we fixed the problem in our code assuming you may do this. 

On April 11 I will release new rf 1.1 with fixes included.

Comment 41

[Steven Michaud [:smichaud] (Retired)]

> you recreate roboform toolbar view (NSView) twice

"You" isn't Firefox.

Is it RoboForm?  Is it the OS?

Is the "roboform toolbar view" a subclass of NSView?  If so which one?

> On April 11 I will release new rf 1.1 with fixes included.

I will try it as soon as it becomes available.  Please let us know here when that happens.

Comment 42

[Steven Michaud [:smichaud] (Retired)]

> On April 11 I will release new rf 1.1 with fixes included.

How's the RF 1.1 release going, Vlad?

The most recent version at http://www.roboform.com/download is still 1.0.7.

Comment 43

[Steven Michaud [:smichaud] (Retired)]

Alex, a decision point for this bug is coming up soon -- the code freeze for FF 12 is tomorrow (2012-04-13).

As I see it, our current choices are:

1) Take one of my patches on the beta branch.

2) Blocklist all Mac versions of RoboForm, including the current one.

Hopefully our range of choices will broaden once RF 1.1 comes out.  But I'll need to first check it thoroughly in my debugging-patched version of FF.

Comment 44

[Vadim Maslov]

Release will come out later today

Comment 45

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 612193 [details] [diff] [review]
Patch (work around dangling pointers)

With the FF 12 code freeze tomorrow, things are getting down to the wire.

Markus, I know you don't like working around extension bugs in FF code.  But (as I've already mentioned) we've done this many times for plugin and OS bugs, so it's a legitimate option.  And RoboForm does have a large number of users.

Let's hope RF 1.1 fixes the bug.  But in case it doesn't, I'd like to have both of these patches ready to go into the tree at a moment's notice.

If you don't have time for the reviews, or if you'd rather not do them, please let me know as soon as possible.  I'll find someone else.

Comment 46

[Steven Michaud [:smichaud] (Retired)]

> Release will come out later today

Thanks!

Please comment here as soon as it does.  And also let us know if we need to get it somewhere else than http://www.roboform.com/download.

Comment 47

[Steven Michaud [:smichaud] (Retired)]

As for the issue I raise in comment #33 (and elsewhere in this bug), nobody's addressed it here, and the one person I asked about it offline hasn't gotten back to me.

If I had to decide now which of my patches to choose, based on the information I currently have, I'd choose the "alternate" one (the one that prevents the RoboForm plugin from being unloaded "prematurely").  I think the chances that doing this would open a security hole in RoboForm are very small, so I'd be willing to take the risk.

I'm quite sure that you need to do more than explicitly unload rf-firefox-plugin to prevent it from leaking sensitive information.

Comment 48

[Vlad Marchenko]

so we just release the fixed version. http://www.roboform.com/dist/roboform-mac.dmg

Comment 49

[Vadim Maslov]

RoboForm Ver 1.1.1 has been released.

It fixes this problem.

Comment 50

[Steven Michaud [:smichaud] (Retired)]

I downloaded RoboForm 1.1.1 from http://www.roboform.com/download and have been testing with it.

I can confirm that it does fix this bug.

Apparently the fix has been accomplished by no longer explicitly unloading the pr-firefox-plugin plugin from the extension.  Now it's (safely) unloaded as all other browser modules are unloaded, under mozJSComponentLoader::UnloadModules(), called (indirectly) from the ScopedXPCCOMStartup destructor.

Comment 51

[Steven Michaud [:smichaud] (Retired)]

We'll still need to blocklist all earlier versions of the Mac RoboForm plugin -- all those before version 1.1.1.

Comment 52

[Steven Michaud [:smichaud] (Retired)]

> Now it's (safely) unloaded as all other browser modules are
> unloaded, under mozJSComponentLoader::UnloadModules(),
> called (indirectly) from the ScopedXPCCOMStartup destructor.

I should have said "Now it's (safely) unloaded as all other JS
components are unloaded ...".

Most browser modules are unloaded by the OS.

Comment 53

[Steven Michaud [:smichaud] (Retired)]

Thanks, Vlad and Vadim, for your work on this.

Comment 54

[Jorge Villalobos [:jorgev] (he/him)]

(In reply to Steven Michaud from comment #51)
> We'll still need to blocklist all earlier versions of the Mac RoboForm
> plugin -- all those before version 1.1.1.

We should look at the new version uptake an how that reflects on crash stats before deciding if this is necessary, and when if necessary.

Comment 55

[Alex Keybl [:akeybl]]

(In reply to Jorge Villalobos [:jorgev] from comment #54)
> (In reply to Steven Michaud from comment #51)
> > We'll still need to blocklist all earlier versions of the Mac RoboForm
> > plugin -- all those before version 1.1.1.
> 
> We should look at the new version uptake an how that reflects on crash stats
> before deciding if this is necessary, and when if necessary.

We'll continue to track this bug for FF12, but we don't plan to take an in-product fix. We may still want to blocklist earlier versions.

Comment 56

[Jorge Villalobos [:jorgev] (he/him)]

What are the current stats for this crash? We need to determine if we want to blocklist the old version.

Comment 57

[Steven Michaud [:smichaud] (Retired)]

These seem to have fallen off to almost nothing on the 12 branch:

https://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A12.0&platform=mac&query_search=signature&query_type=contains&reason_type=contains&date=04%2F26%2F2012%2017%3A50%3A12&range_value=1&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=libobjc.A.dylib%400xb350
https://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A12.0&platform=mac&query_search=signature&query_type=contains&query=AppKit&reason_type=contains&date=04%2F26%2F2012%2018%3A01%3A46&range_value=1&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=AppKit%400xd7851

I really wish we could search on Socorro by "-[BaseWindow dealloc]".  But since that's never at the top of any of this bug's stacks, we can't -- which is the subject for another bug (soon to be opened).

Comment 58

[Jorge Villalobos [:jorgev] (he/him)]

It sounds like it's not worth blocklisting, but we can softblock if someone feels this is necessary.