DllInterceptor shouldn't malloc and init locks in static initializers

RESOLVED FIXED in Firefox 62

Status

()

enhancement
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: dmajor, Assigned: aklotz)

Tracking

unspecified
mozilla62
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(1 attachment)

Reporter

Updated

a year ago
Blocks: 1432653
Assignee

Comment 1

a year 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
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

a year 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

a year 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

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/993f78b4700d
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.