Current OpenInputStream implementations use AsyncRead underneath which requires the returned stream to be read on a thread other than the one which called OpenInputStream.

Any thoughts on how to fix this? Maybe use a helper thread under the covers? /be

nice idea! Yea, Open*Stream() could spin a new thread and do basically the exact same thing (use SyncStreamListener()) that it does today, except that it would be done on the helper thread. This helper thread would be able to handle the incoming events from the transports, and the calling thread could block (as desired) until data became available via the threadsafe pipe. Our thread usage would obviously grow which would suck, but this would solve our overall problem.

notes: Here's what I'm thinking. new class nsSyncThread which is a nsIRunnable and nsIThread nsSyncThread::Init(nsIChannel *channel, nsIStreamListener *listener) { mListener = listener; mChannel = channel; } nsSyncThread::Run() { // call async read on this thread so // events are posted here instead of the ui thread. mChannel->AsyncRead(mListener, ...); while(running) { event = waitForEvent(); handleEvent(event); } } httpchannel::OpenInputStream(nsIInputStream**_retval) { nsIStreamListener *listener; NS_NewSyncStreamListener(_retval, nsnull, &listener); ... nsSyncThread *syncher = new nsSyncThread(); syncher->Init(this, listener); newThread(&thread); thread->Dispatch(syncher); return rv; }

I've attatched the helper implementation and how it should be used in HTTP. you have to add the helper file to the make file so it's built. I haven't tested these though they "should" work.

Cc'ing pete so he can try the patches out too. /be

And cc'ing mj@digicool.com, who wants this for synchronous xml-rpc calling. /be

This bug is most likely the cause of bugs 28466 and 33542. Because of that, I am nominating this for nsbeta2.

I was afraid this was going to happen. Can I retract the patches :-). If these patches work (the http channel part probably needs to be whacked a bit), they do not give sync reading carte blanc to the world. Syncronous reading on the UI thread is VERY BAD because it halts event processing and UI transitioning. cc'ing ruslan in hopes that he can take this bug over :-)

Bad or not, currently peoply using sync calls don't even get to find out it is bad for the ui thread. Things just hang for ever. The API to do sync calls is there, being used, and broken, also for calls from non-ui thread.

CC-ing gagan since I'll be out of town for 4 days. Yes. Unfortunately http handler is designed so it won't allow correct sync operations on the same thread. So, Jud - do you want these patches applied?

yea, if someone could run w/ these patches that would be great.

I'm starting to look into the wallet problem and I has nothing to do with http handler not being able to do synchronous operations. The wallet thread actually creates it's own event queue and does AsyncRead. So these 2 bugs are unrelated the best I can tell.

Putting on [nsbeta2+] radar for beta2 fix. Assigning to ruslan per valeski.

I vote not to fix this for this release. The bug by itself doesn't block anything (dependencies listed are incorrect here). The synchronous client can implement it manually (following wallet's example). Doing it under the cover by creating the additional thread in the handler is not a clean solution. We'll address this in necko2.0 by providing trully sycnhronous way of doing this. Gagan, your decision?

From JS I can't create a seperate thread. I can't work around this bug.

Ok. Though even as I believe that it's not critical for Beta as it won't fix any real issues anyway - I'm still going to fix this.

Fixed now along the lines of the original Jud's patch

verified: winNT 2000070508