Closed Bug 17262 Opened 25 years ago Closed 7 years ago

xpconnect Release calls on target object need to happen on correct thread.

Categories

(Core :: XPConnect, defect, P3)

x86
All
defect

Tracking

()

RESOLVED WORKSFORME
Future

People

(Reporter: jband_mozilla, Assigned: dbradley)

Details

From the xpcom newsgroup... Subject: Re: [xpconnect] support only one JSRuntime? Date: Tue, 26 Oct 1999 11:38:12 -0700 Scott Furman wrote: > > John Bandhauer wrote: > > > I'd like to make and enforce the rule that xpconnect will > > only > > support one JSRuntime in a process. I want to find out if > > anyone > > objects. > > Isn't it true that some non-UI threads do make calls into JS ? > (XPInstall, profiles, Java threads [via LiveConnect]). I see a > potential problem with this... > > My understanding of XPConnect may be flawed, so keep me in > line: When the last reference by JS to an XPCOM object is > dropped (because the last reference to one of its JS > interface-wrapper objects has been dropped), a subsequent GC > will cause the JS wrapper object to be finalized and the > underlying XPCOM interface to be Release'ed. > > The problem with this is that JS garbage collection can take > place on any thread that calls into JS, and the Release() calls > to the native XPCOM interface will be made on the thread that > does GC. Most interfaces expect to be called on only a single > thread and cannot safely deal with a Release() call made from > another thread. Damn Scott. I was having a perfectly nice morning in blissful ignorance of this problem... > > I'm certain that we don't want to require every XPConnect'able > interface to be thread-safe in its Release() call. Here are a > few ideas to kick around: > > * Enhance XPConnect to work with multiple JSRuntime's. > Restrict the use of the JSRuntime to a single thread (not > necessarily the UI thread). In this way, for example, > XPInstall could use its own thread and the rest of the > browser can operate on the UI thread. I don't think that this will fly in general. This makes JS objects too much of a second class citizen. "You can't get there from here". > * Restrict XPConnect to a single JSRuntime, but impose the > single-threaded apartment model on calls to XPCOM > interfaces. XPConnect would automatically proxy Release() > calls to objects when called on a thread other than the > one in which they were created. I basically agree with this. However, the mechanism for proxying the Release calls I envision might be different from what one might imagine. My thought is to queue up a list of objects which need to be 'Released' in thread local storage inside xpconnect and then the next time xpconnect gets called on the thread in question it would 'unload' that queue by doing a whole sequence of release calls. Of course the normal case is that gc gets called on the 'right' thread and the Release call is immediate. Does this make sense? So, this is yet another class of bookkeeping that xpconnect has to do. Oh boy. While xpconnect can 'proxy' its own (internally generated) Release calls, I don't think that it should attempt to proxy arbitrary calls made from (or to) JS code. It is up to the callers to get their threading schemes in order. > * Limit GC to the UI thread. This could lead to deadlock > situations in which a non-UI thread completely exhausts > memory and suspends, but the UI thread is not runnable > and, therefore, it cannot perform GC to reclaim memory. > It's possible that, by running GC often enough on the UI > thread, we can make this deadlock situation so unlikely > that it is not an issue. You saved me pointing out the problem with this by doing so yourself first. It is worth noting that our xpcom threading system quite is sloppily defined. Though we have a mechanism for proxying we have an implicit assumption that almost all code runs on the UI thread. Code on other threads is expected to jump through hoops. Services that are not thread safe are playing with fire (I'm ashamed to admit that xpconnect is currently still in this class!) This is quite a mess. John.
Status: NEW → ASSIGNED
Moving all XPConnect QA contact to rginda
QA Contact: cbegle → rginda
Note that a while ago, I made the DOM set a GC callback that returns false for the "begin" phase, defeating the GC attempt, if called on any thread other than the DOM (main) thread. So we're now in the world fur described where a non-main thread could exhaust GC space and deadlock waiting for the main thread to run the GC, while the main thread waits for the other thread. "Don't go there." The main thread is more and more resembling the classic codebase's "mozilla thread" -- it must not block (for long, if at all) waiting for some other thread. /be
mass reassign of xpconnect bugs to dbradley@netscape.com
Assignee: jband → dbradley
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Target Milestone: --- → Future
Removing Rob as QA contact
QA Contact: rginda → pschwartau
QA Contact: pschwartau → xpconnect
XPConnect has been single threaded with lots of threading checks for quite a while now.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.