Closed
Bug 1332523
Opened 8 years ago
Closed 8 years ago
Various cleanup before full fledged refactoring of the bootstrap API
Categories
(Toolkit :: Startup and Profile System, defect)
Toolkit
Startup and Profile System
Tracking
()
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: glandium, Assigned: glandium)
References
(Blocks 1 open bug)
Details
Attachments
(5 files)
(deleted),
text/x-review-board-request
|
away
:
review+
|
Details |
Bug 1332523 - Make the Bootstrap API entry point the same for both dependent and standalone linkage.
(deleted),
text/x-review-board-request
|
benjamin
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
benjamin
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
benjamin
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
benjamin
:
review+
|
Details |
I have some patches that can land independently of more definite changes on the bootstrap API.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment on attachment 8828609 [details]
Bug 1332523 - Move message indicating when the blocklist is initialized after user32.dll was loaded to the blocklist itself.
https://reviewboard.mozilla.org/r/105936/#review106902
Attachment #8828609 -
Flags: review?(dmajor) → review+
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8828610 [details]
Bug 1332523 - Make the Bootstrap API entry point the same for both dependent and standalone linkage.
https://reviewboard.mozilla.org/r/105938/#review107094
r=me with s/static/inline/ or please rerequest review if that's not possible/good.
::: toolkit/xre/Bootstrap.h:137
(Diff revision 1)
> +#else
> extern "C" NS_EXPORT void NS_FROZENCALL
> XRE_GetBootstrap(Bootstrap::UniquePtr& b);
> -typedef void (*GetBootstrapType)(Bootstrap::UniquePtr&);
>
> +static Bootstrap::UniquePtr
static in a header seems bad. Why not just `inline`?
Attachment #8828610 -
Flags: review?(benjamin) → review+
Comment 8•8 years ago
|
||
mozreview-review |
Comment on attachment 8828611 [details]
Bug 1332523 - Remove nsXPCOMGlue.h.
https://reviewboard.mozilla.org/r/105940/#review107096
Attachment #8828611 -
Flags: review?(benjamin) → review+
Comment 9•8 years ago
|
||
mozreview-review |
Comment on attachment 8828612 [details]
Bug 1332523 - Make GetBootstrap take the path to an arbitrary file next to libxul.
https://reviewboard.mozilla.org/r/105942/#review107098
Attachment #8828612 -
Flags: review?(benjamin) → review+
Comment 10•8 years ago
|
||
mozreview-review |
Comment on attachment 8828613 [details]
Bug 1332523 - Add BinaryPath::Get variant that returns a UniquePtr instead of filling a stack buffer.
https://reviewboard.mozilla.org/r/105944/#review107100
r=me either way with a slight preference for not adding a new FreePolicy class.
::: xpcom/build/BinaryPath.h:158
(Diff revision 1)
>
> #else
> #error Oops, you need platform-specific code here
> #endif
>
> + struct FreePolicy { void operator()(void* p) { free(p); } };
http://searchfox.org/mozilla-central/source/mfbt/UniquePtrExtensions.h#43 already has a FreePolicy implementation. Is it better to repeat that here rather than use the existing one?
Attachment #8828613 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 11•8 years ago
|
||
Heh, I wasn't aware of the UniquePtrExtensions.h header.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 18•8 years ago
|
||
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/beb1258e4871
Move message indicating when the blocklist is initialized after user32.dll was loaded to the blocklist itself. r=dmajor
https://hg.mozilla.org/integration/autoland/rev/7402945cf573
Make the Bootstrap API entry point the same for both dependent and standalone linkage. r=bsmedberg
https://hg.mozilla.org/integration/autoland/rev/bb2b50d94e76
Remove nsXPCOMGlue.h. r=bsmedberg
https://hg.mozilla.org/integration/autoland/rev/e1baf4f6ef96
Make GetBootstrap take the path to an arbitrary file next to libxul. r=bsmedberg
https://hg.mozilla.org/integration/autoland/rev/8c3f02820491
Add BinaryPath::Get variant that returns a UniquePtr instead of filling a stack buffer. r=bsmedberg
Comment 19•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/beb1258e4871
https://hg.mozilla.org/mozilla-central/rev/7402945cf573
https://hg.mozilla.org/mozilla-central/rev/bb2b50d94e76
https://hg.mozilla.org/mozilla-central/rev/e1baf4f6ef96
https://hg.mozilla.org/mozilla-central/rev/8c3f02820491
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment 20•8 years ago
|
||
Yesterday, after this bug had landed, I tried to compile Thunderbird (after integrating the changes here in bug 1332898) with
ac_add_options --disable-sandbox
in my Mozconfig (which was really an undesired left-over).
The result was that TB didn't start, this was the stack:
mozglue.dll!arena_dalloc(void * ptr, unsigned __int64 offset) Line 4709 C
thunderbird.exe!mozilla::GetBootstrap(const char * aXPCOMFile) Line 417 C++
thunderbird.exe!InitXPCOMGlue(const char * argv0) Line 248 C++
thunderbird.exe!NS_internal_main(int argc, char * * argv, char * * envp) Line 297 C++
thunderbird.exe!wmain(int argc, wchar_t * * argv) Line 118 C++
I debugged it a bit and strangely it failed on the 'return' (!) statement here after successfully returning from (*func)(b):
https://dxr.mozilla.org/mozilla-central/rev/3cedab21a7e65e6a1c4c2294ecfb5502575a46e3/xpcom/glue/standalone/nsXPCOMGlue.cpp#407
On the frozen console (couldn't copy/paste) I saw:
jemalloc.c:4710: Failed assertion: "arena != NULL"
Hit MOZ_CRASH() at jemalloc_config.cpp:163
So something went wrong in memory management.
I haven't tried this in Firefox, since it would cost me one hour to compile the potentially failing version and then another hour to return to a good version. However, TB and FF have a very similar main() so I assume it FF wouldn't work either.
If anyone is interested, I could build FF, or the interested party could do this. I'm not sure whether this option is still supported, if not, it should be removed instead of causing non-functioning builds.
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(benjamin)
Comment 21•8 years ago
|
||
I cannot explain that, but it should be filed and diagnosed in a different bug. It sounds like something that could be figured out with a debugging session.
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(benjamin)
You need to log in
before you can comment on or make changes to this bug.
Description
•