Closed
Bug 1463961
Opened 6 years ago
Closed 6 years ago
DllInterceptor shouldn't malloc and init locks in static initializers
Categories
(Core :: XPCOM, enhancement)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: away, Assigned: bugzilla)
References
Details
Attachments
(1 file)
6.92 KB,
patch
|
handyman
:
review+
|
Details | Diff | Splinter Review |
Assignee | ||
Comment 1•6 years ago
|
||
We can get rid of the allocation and mapping easily enough; that stuff was really only there for sharing trampoline space with cross-process interceptors, which I have decided that I don't want to support anyway. It also gives us the added bonus of removing an extra level of indirection. As for the CS, that one is harder to steer around. I'd switch it to SRWLOCKs if it wasn't for the fact that we actually depend on recursive entry. I think the real question at hand is, is there any good reason to statically initialize those blocklist interceptors in the first place? In general our coding style guide bans static initializers, so while I'm happy to clean up some of this code for the other benefits that it brings, I'm more inclined to suggest that, if we're concerned enough about the static initializers in mozglue, we should just switch those interceptor instantiation sites to use dynamic allocation.
Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•6 years ago
|
||
Attachment #8981280 -
Flags: review?(davidp99)
Comment 3•6 years ago
|
||
Comment on attachment 8981280 [details] [diff] [review] Make shared VM policy only work with in-proc memory policies and remove pid mapping Review of attachment 8981280 [details] [diff] [review]: ----------------------------------------------------------------- I'd nitpick the use of the magic number 4000 in the InitializeCriticalSectionEx call but we do it in a number of places and its not hard to find in the docs so why bother. I'm a bit weirded out by all of the static WindowsDllInterceptors we have. But I think I agree that finding a proper place to early-init them would be more trouble than its worth.
Attachment #8981280 -
Flags: review?(davidp99) → review+
Assignee | ||
Updated•6 years ago
|
Summary: DllInterceptor shouldn't malloc and take locks in static initializers → DllInterceptor shouldn't malloc and init locks in static initializers
Assignee | ||
Comment 4•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/993f78b4700d242461c4e78c252b7a00d14bfd9f Bug 1463961: DLL Interceptor - Make shared VM policy only work for in-proc interceptors and remove pid mapping; r=handyman
Comment 5•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/993f78b4700d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in
before you can comment on or make changes to this bug.
Description
•