Closed Bug 16773 Opened 21 years ago Closed 14 years ago

Enable AutoProxification

Categories

(Core :: XPCOM, defect, P3, critical)

defect

Tracking

()

RESOLVED WONTFIX
Future

People

(Reporter: dougt, Unassigned)

References

Details

(Whiteboard: [nocrash])

Attachments

(1 file, 2 obsolete files)

with autoproxification turned ON, opening a file OnProgress crashes.
Assignee: dp → dougt
Doug did you mean to file this on me. Reassign if you really meant to.
no this is mine.
Target Milestone: M15
I have autoproxification turned off.  It will remain this way until coclasses
are invented.
Adding "crash" keyword to all known open crasher bugs.
Keywords: crash
moving to m20.
Target Milestone: M15 → M20
changing summary. adjusting milestone
Summary: With Autoproxy turn ON, opening a file OnProgress crashes. → Enable AutoProxification
Target Milestone: M20 → Future
*** Bug 16678 has been marked as a duplicate of this bug. ***
Looks like a dup of bug 53080.
dp is no longer @netscape.com. changing qa contact to default for this product
QA Contact: dp → kandrot
Marking [nocrash] because the crash happened with autoproxification turned ON,
and autoproxification has been turned OFF per Doug T 2000-07-26 19:02.
Whiteboard: [nocrash]
Oops, sorry, I meant per Doug T 1999-10-21 14:46. Please excuse the spam.
Keywords: crash
uhm, what in the world is "autoproxification"?  is this bug still valid?
Doug: 
Can you remember what the issues were here?
What do you mean by comment #3?
i was hoping to automatically create wrappers for all out val, as well as when
anyone created a component or services which was marked in some special way via
the nsIClassInfo (then called a coclass).
(In reply to comment #15)
> i was hoping to automatically create wrappers for all out val, as well as when
> anyone created a component or services which was marked in some special way via
> the nsIClassInfo (then called a coclass).

How does nsIClassInfo fit into this? Isn't interface info all you need here? I
thought this would just be a matter of looking at the parameter list after
calling XPTC_InvokeByIndex() and wrapping all interface pointers there, but I'm
sure sure I'm missing something...
nsIClassInfo tells you if the class is threadsafe or not.  If it is threadsafe
then there is no need to create a proxy of an instance of the class.
That it can be called on multiple threads doesn't necessarily mean that it can
and should be called on _any_ thread, does it?  Would you see necko ever
deciding to call its listeners on the HTTP threads just because they were marked
threadsafe?
That's a good question.  I don't know.  Certainly, there are contracts in place
for certain interfaces that require callbacks (listeners) to be invoked on
specific threads (for example, nsIStreamListener's methods are called on the
thread that called AsyncOpen).

I think it would be useful to have a way to say give me a proxy for this object
and any objects that it may return via method calls.  For example, the
notification callbacks passed to a nsIChannel might need to be accessed from a
background thread.  Necko would like to be able to get a proxy for a
nsIInterfaceRequestor that automatically wraps a proxy around objects returned
from GetInterface, so that implementors of nsIInterfaceRequestor do not need to
care about the fact that their ultimate consumer may live on a background
thread.  However, if an interface returned from GetInterface is already
threadsafe, then it would seem that the consumer wouldn't mind being called from
an arbitrary thread.  Make sense?
classinfo sometimes also answers the QI question which should make life easier...
(In reply to comment #17)
> nsIClassInfo tells you if the class is threadsafe or not.  If it is threadsafe
> then there is no need to create a proxy of an instance of the class.

Ok, so nsIClassInfo could help making this more efficient, but I think
autoproxification would be useful even if everything got unconditionally proxified.
Apart from potential performance issues, the only technical problem I can think
of is QueryInterface implementations that return a proxied object themselves and
land us in an infinite loop.
the infinite loop can be solved -- each wrapper response positively to a UUID
when QI'ed indicating that it is really a wrapper.

I think the first step here is to design what we are going to do with respect to
auto proxification.  

A few requirement may be:
* ensure that calls through proxies automatically wrap any nsISupport params.

* have a CI that honors some co-class information about how the object should be
created.  in my Microsoft lingo -- the classes apartmenting type.
It would be nice to have all the COM Class ThreadingModel stuff, but how about
ignoring this for the time being and assuming that in many cases an object will
want access restricted to the same thread as itself for any interfaces it dishes
out (be they interfaces to itself or to other objects)?
It looks like this should be pretty easy to implement and it would be very
useful for many scenarios (in fact i have an immediate use for this in the media
streaming lib i'm coding at the moment). We could make it an optional behaviour
by adding a PROXY_AUTOPROXIFY flag to GetProxyForObject().
As a second step we could honour some nsIClassInfo tagging to prevent
proxification of certain objects.

Alex: What about nsIServiceManager::GetService?  and nsIComponentManager::
CreateInstance?  Those functions return objects that may only expect to be used
on the main thread.  What if someone attempts to access them from a background
thread via those methods?  I think that's where auto-proxification will be very
handy, and it would need to be triggered off of something like nsIClassInfo or
perhaps something in the component registry.  That said, I agree that simply
extending NS_GetProxyForObject to auto-proxify returned nsISupports objects
would be a good start and help in many cases.
what darin said.  

if you want to jump in and make sometihng work, making inout and outval's be
proxied when using a proxy would be a big help.
(In reply to comment #24)
> Alex: What about nsIServiceManager::GetService?  and nsIComponentManager::
> CreateInstance?  Those functions return objects that may only expect to be used
> on the main thread. 

Absolutely. I agree it would be nice to have auto-proxification here, but I
really see this as a separate issue.
For the time being there are workarounds to enforce that objects are created on
specific threads (e.g. on the client side by accessing the service or component
managers via a proxy; on the factory side by writing a suitable factory method
that instantiates an object on some thread and returns a proxy).

(In reply to comment #25)
> if you want to jump in and make sometihng work, making inout and outval's be
> proxied when using a proxy would be a big help.

Ok, I'll take a stab at this.
Attached patch patch v1 (obsolete) — Splinter Review
This patch adds 2 new flags for GetProxyForObject():

PROXY_AUTOPROXIFY: 
Proxies all 'in' and 'out' interface parameters unless they are already
proxified. 'inout' interface parameters will not be proxified, mainly because
we would need to store some extra state during the call to get this right. I
don't know whether we want to do that given that 'inout' interface pars are not
very common.

PROXY_ISUPPORTS: 
If this flag is set, all AddRef/Release/QI calls will be proxied, so that
objects with non-thread-safe nsISupports implementations can be proxied.
Usually only the final Release() is proxied to the object's thread. 


Doug: Care to review this patch?
Blocks: 317491
Attached patch patch v.2 (obsolete) — Splinter Review
patch v.2 adds contains the functionality of patch v.1 and adds a couple of new flags:

- PROXY_SAMETHREAD: calls are proxied even if the caller is on the same thread

- PROXY_NESTED_QUEUES: pushes a new event queue on the calling thread when making calls through the proxy. This ensures that the caller only handles events originating from the callee while waiting for a synchronous call to complete.

The patch also implements tagging of event queues with the (proxy) thread that they are accepting events for: When a proxy posts an event it looks for the youngest queue that is accepting events for it. This ensures that proxies with PROXY_NESTED_EVENT_QUEUES set don't deadlock in 3-way (or more complex) call scenarios.
Attachment #189047 - Attachment is obsolete: true
Blocks: 29474
QA Contact: kandrot → dougt
Assignee: dougt → nobody
QA Contact: dougt → xpcom
Comment on attachment 208841 [details] [diff] [review]
patch v.2

This patch is quite stale now. I have a newer patch that works with Darin's thread code rewrite, but after struggeling with autoproxied code for a year or so, I'm finally convinced now that they are too blunt an instrument to be useful in practice. IMO we should close as WONTFIX.
Attachment #208841 - Attachment is obsolete: true
No longer blocks: 317491
I agree.  our interprocess D-XPCOM is manual.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → WONTFIX
could you possibly post your last changeset (cvs diff or hg bundle)?

i'd like to think about this again...
(In reply to comment #31)
> could you possibly post your last changeset (cvs diff or hg bundle)?
> 
> i'd like to think about this again...

Unfortunately I haven't got a patch. You could try to piece it together from the zap changelogs: http://bonsai.mozilla.org/cvsquery.cgi?branch=ZAP_20050610_BRANCH&date=all 

I removed the functionality in checkins dated "2007-03-02 02:42" and "2006-11-28 05:47", so these will give you the most up-to-date patches that I have, albeit reversed.

Some words on why we stepped away from autoproxies in zap. We did use them quite a bit and it works great if there are only 2 threads involved. Things get messy when there are 3 or more threads that communicate among each other using synchronous proxies.

E.g. in the zap case, we had a network thread and a media processing thread talking to the main UI thread using synchronous proxies and vice versa. In this case there is a race condition: When the UI thread makes a synchronous proxy call to the media processing thread and waits for the result, a synchronous call from the network thread might slip in, messing up proper call sequencing on the UI thread. 

I contemplated tagging synchronous calls in some way, so that only proxy calls in the same "trace" get handled by an event queue that is waiting for a proxy call to complete. The problem is that this leads to all sorts of deadlock scenarios when there are mutual calls between more than 2 threads.

The current scheme that we're using in zap is to use synchronous proxies to call from the main thread into background threads, and asynchronous proxies in the other direction. This is somewhat limiting in that background threads cannot request and wait for data from the main thread within the same function call, but it fixes the sequencing and deadlock issues. It also means that we don't block background threads for unpredictable durations, which is usually what you want. E.g. in zap we have an audio thread that needs to feed a buffer every 20ms - performing a synchronous proxy call from this thread to the main UI thread is not a particularly good idea :-)
so, i got tired and somehow missed your response.

yeah, the major problem with proxies if you manage to get them to work is that either you run event loops or you have to work about getting twisted proxies.

I'll have to check to see which way proxies work in trunk wrt pushing event queues.

this is an implementation and a partial testcase driver.

basically:
load("proxifier.js") // load this script
work() // lets the thread work
cleanup() // kills the threads
quit() // exits
You need to log in before you can comment on or make changes to this bug.