Closed Bug 1076677 Opened 10 years ago Closed 6 years ago

JavaScript strict warning: , line 0: TypeError: setting getter-only property "Services"

Categories

(Core :: XPConnect, defect)

All
Linux
defect
Not set
normal

Tracking

()

RESOLVED INACTIVE

People

(Reporter: ishikawa, Unassigned)

References

Details

During the run of |make mozmill| test sute using DEBUG BUILD of TB (c-c)
I get many (3K+) lines of the following warning

JavaScript strict warning: , line 0: TypeError: setting getter-only property "Services"

This should be investigated and fixed.
(I think that this "Services" appears so many times suggests some
improper initialization for "Services".
That the message fails to provide the file name and line # is also a suspect.)

cf. Bug 1067942 - "TypeError: setting a property that has only a getter" without mentioning file and property name

There is one other warning of similar type, and I am filing another bugzilla for
the following:

JavaScript strict warning: chrome://messenger/content/specialTabs.js, line 530: TypeError: setting getter-only property "loadingTabString"
Blocks: 826732
See Also: → 1067942
The warning is printed after I applied the work-in-progress patch in Bug 1067942 
(without the patch, I don't get the name of "Services".)

TIA
See Also: → 1076683
Depends on: 1067942
See Also: 1067942
This warning appears so many times that this must be some bug in a core file that is included almost everywhere. So we must find out where it is, unless the warning says the exact file (that should be asked for in bug 1067942).

E.g. I have determined 1 of these warnings appears when opening tools->Options->Composition in TB. Not every pane in Options produces it. So that is a step and we could start tracing it from there.
I can reproduce this by setting a breakpoint in js_ReportGetterOnlyAssignment:

    $ ./mach build
    $ ./mach mochitest --debugger=gdb mochitest/general/test_showModalDialog.html
    ...
    (gdb) break js_ReportGetterOnlyAssignment
    Function "js_ReportGetterOnlyAssignment" not defined.
    Make breakpoint pending on future shared library load? (y or [n]) y
    Breakpoint 1 (js_ReportGetterOnlyAssignment) pending.
    (gdb) r
    ...
    ...
    ...
    Breakpoint 1, js_ReportGetterOnlyAssignment (cx=0x7ffff6c94190, strict=false)
        at /home/jorendorff/dev/gecko/js/src/jsobj.cpp:3996
    3996                                            JSMSG_GETTER_ONLY);


I think it happens when a file does something like

    XPCOMUtils.defineLazyModuleGetter(this, "NewTabUtils",
                                      "resource://gre/modules/NewTabUtils.jsm");

and then something else does

     Cu.import("resource://gre/modules/NewTabUtils.jsm");

For example, browser.jsm does the first part and then #includes browser-thumbnails.js, which does the second part.

Regardless of what else is going on here, it'd be nice to understand why the error message is so useless.
It's likely there's other code triggering the same warning too. Anyway, in this particular case, the warning could be eliminated by

1. changing mozJSComponentLoader::ImportInto to use JS_DefinePropertyById rather than JS_SetPropertyById

or by

2. changing XPCOMUtils.defineLazyGetter to also define a setter:

        defineLazyGetter: function XPCU_defineLazyGetter(aObject, aName, aLambda)
        {
          Object.defineProperty(aObject, aName, {
            get: function () {
              delete aObject[aName];
              return aObject[aName] = aLambda.apply(aObject);
            },
+           set: function (v) {
+             delete aObject[aName];
+             aObject[aName] = v;
+           },
            configurable: true,
            enumerable: true
          });
        },

Separately, XPConnect is doing something (on entry to Cu.import(), I think) that prevents the JS engine from recovering the file and line number to produce a decent error message. If I select the js::Invoke frame using `(gdb) frame 14` and then do `(gdb) finish` and `(gdb) call js_DumpBacktrace(cx)`, it does print a stack with a file and line. However, whatever shenanigans XPConnect gets up to, it's super frustrating that our JS stack-walking code can't cope with this when asked from GDB. That's a JS engine bug.

->XPConnect. Maybe Bobby will take a look at the warnings some rainy day.
Component: JavaScript Engine → XPConnect
Flags: needinfo?(bobbyholley)
But why would only "Services" be producing the warning? There is a ton of similar "objects" in various modules (.jsm).
(In reply to :aceman from comment #5)
> But why would only "Services" be producing the warning? There is a ton of
> similar "objects" in various modules (.jsm).

It'll only trigger if some code is doing both XPCOMUtils.defineLazyModuleGetter() and Cu.import(), for the same module, in the same global.

This isn't super common in the browser and I imagine it might be even less common in Thunderbird, which doesn't appear to use defineLazyModuleGetter at all.
(In reply to Jason Orendorff [:jorendorff] from comment #4)
> It's likely there's other code triggering the same warning too. Anyway, in
> this particular case, the warning could be eliminated by
> 
> 1. changing mozJSComponentLoader::ImportInto to use JS_DefinePropertyById
> rather than JS_SetPropertyById
> 
> or by
> 
> 2. changing XPCOMUtils.defineLazyGetter to also define a setter:
> 
>         defineLazyGetter: function XPCU_defineLazyGetter(aObject, aName,
> aLambda)
>         {
>           Object.defineProperty(aObject, aName, {
>             get: function () {
>               delete aObject[aName];
>               return aObject[aName] = aLambda.apply(aObject);
>             },
> +           set: function (v) {
> +             delete aObject[aName];
> +             aObject[aName] = v;
> +           },
>             configurable: true,
>             enumerable: true
>           });
>         },
> 

Would you mind proposing a patch on this?
[I don't understand the core JS stuff at all, but am frustrated like others.]

> Separately, XPConnect is doing something (on entry to Cu.import(), I think)
> that prevents the JS engine from recovering the file and line number to
> produce a decent error message. If I select the js::Invoke frame using
> `(gdb) frame 14` and then do `(gdb) finish` and `(gdb) call
> js_DumpBacktrace(cx)`, it does print a stack with a file and line. However,
> whatever shenanigans XPConnect gets up to, it's super frustrating that our
> JS stack-walking code can't cope with this when asked from GDB. That's a JS
> engine bug.
> 

This is very bad, too, from the developers' point of view.

> ->XPConnect. Maybe Bobby will take a look at the warnings some rainy day.

Hope someone will pitch in to solve this in not so distant future.

TIA
Why do many places do defineLazyModuleGetter(Services.jsm) instead of Cu.import(Services.jsm) ?
What is the advantage?
(In reply to :aceman from comment #8)
> Why do many places do defineLazyModuleGetter(Services.jsm) instead of
> Cu.import(Services.jsm) ?
> What is the advantage?

IIRC, If you eagerly load the jsm, then you end up with a compartment per jsm, even for things we don't use. It isn't a lot of memory for each one, but it adds up.
(In reply to Jason Orendorff [:jorendorff] from comment #4)
> ->XPConnect. Maybe Bobby will take a look at the warnings some rainy day.

When I'm finished overhauling the error reporting infrastructure perhaps.
Depends on: 981187
Flags: needinfo?(bobbyholley)
(In reply to Andrew McCreight [:mccr8] from comment #9)
> (In reply to :aceman from comment #8)
> > Why do many places do defineLazyModuleGetter(Services.jsm) instead of
> > Cu.import(Services.jsm) ?
> > What is the advantage?
> 
> IIRC, If you eagerly load the jsm, then you end up with a compartment per
> jsm, even for things we don't use. It isn't a lot of memory for each one,
> but it adds up.

Does the suggested patch in comment 7 to
XPCOMUtils.defineLazyGetter
also end up with a compartment per jsm?
If NOT, maybe I might want to try to patch XPCOMUtils.defineLazyGetter and
see what will become of the warnings.
Even if this patch applies to the particular case according to 

> Anyway, in this particular case, the warning could be eliminated by ...

I would like to see 3K lines of such warnings removed from the log of 
|make mozill| test of C-C TB debug run.
These lines add to the already cluttered mess of so many warning lines further.

That XPCOM inhibits the proper file and line number for warning is very frustrating, but
I think that can wait for now: it seems to have a such a deep cause and digging up and
figuring out how to fix seems a very long term project, indeed :-(
The message has suddenly stopped showing in today's nightly. Has anything changed in m-c?
Bug 1113369 and a bunch of other related bugs were fixed recently.  There's a bunch of post-landing cleanup/fixes happening right now, tho, with high priority, so I doubt any investigation into this will happen any time soon to verify whatever happened was as desired.
I have not seen the useless error message for like a few weeks now.
At the same time, I do get meaningful Type Error, etc.
Thank you for the improvement.

If I don't see the useless messages at the end of April,  I will close this bug.
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.