NetUtil.asyncCopy should print a big fat warning when using non-blocking streams. Adding checks around http://dxr.mozilla.org/mozilla-central/source/xpcom/io/nsStreamUtils.cpp#l253 might be sufficient, assuming interfaces dont lie about being async

Currently there is a lot of misinformed asyncCopy usage, eg https://bugzilla.mozilla.org/show_bug.cgi?id=562674

Mike or David, one of you guys should own fixing this

We could do it in JS or in C++. I am inclined to say JS, using Deprecated.jsm, because this will let devs find the culprit more easily.

We should do this in C++, because this pattern is used in C++ too. Technically it's not deprecated since os.file isn't a complete alternative(eg it doesn't copy from network streams, zips)

I believe that we are going to have lots of false positives/negatives on that one. Nothing ensures that |nsIAsync{Input, Output}Stream| implementations don't block the main thread, just as nothing ensures that |isNonBlocking()| streams don't block the main thread. I'll write a first version, but I suspect that we're going to need some fine-tuning. Side-note: Deprecated.jsm doesn't mean that OS.File can replace it, just that devs should find a better implementation. I'd be inclined to have both the C++ version to get the warning and the JS version as it prints the stack.

We'll also need to update the documentation of NetUtil.asyncCopy, which gives an anti-pattern example.

Adding a warning also at JS level. This is essentially the same warning as in C++, except it is conditioned upon "devtools.errorconsole.deprecation_warnings" instead of DEBUG and it also displays the JS stack.

Looking at the code more carefully, I have the impression that NS_AsyncCopy actually dispatches every call to read or write off the main thread (argument |target|), regardless of whether the thread is sync or async - that's methods DoCopy() and Process() in nsAStreamCopier and its subclasses. Also, closing seems to be done OMT. If I'm right, I can't think of any meaningful warning we could add in nsStreamUtils.cpp. Taras, am I missing something?

Comment on attachment 816514 [details] [diff] [review] Warn when calling NS_AsyncCopy with a non-nsIAsync*Stream. Canceling r? until I have investigated the underlying issue.

Comment on attachment 816515 [details] [diff] [review] Warn when calling NetUtil.asyncCopy with a non-nsIAsync*Stream. Canceling r? until I have investigated the underlying issue.

Calling NetUtil.asyncCopy on a (buffered) nsIJARInputStream has the following effect: - it fills the buffer onces on the main thread during the call to |ioUtil.inputStreamIsBuffered|; - the subsequent I/O operations are performed off the main thread. Note that we cannot create it on an unbuffered nsIJARInputStream (see bug 927366).

Taking care of that single main thread I/O in bug 927407.

> Looking at the code more carefully, I have the impression that NS_AsyncCopy > actually dispatches every call to read or write off the main thread > (argument |target|), regardless of whether the thread is sync or async - > that's methods DoCopy() and Process() in nsAStreamCopier and its subclasses. > Also, closing seems to be done OMT. > > If I'm right, I can't think of any meaningful warning we could add in > nsStreamUtils.cpp. > > Taras, am I missing something? Sounds like you found some main thread IO and some evidence that most IO is off main thread. Happy to hear, this wasn't as bad as I thought.

Doesn't seem that there is much we can do here. Closing the bug.