Open
Bug 227113
Opened 21 years ago
Updated 2 years ago
Streamconverters don't deal well with Suspend/Resume
Categories
(Core :: Networking, defect, P5)
Core
Networking
Tracking
()
NEW
Future
People
(Reporter: Biesinger, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [necko-would-take])
If a streamlistener suspends the request in OnStartRequest, an existing
streamconverer does not know about that... and some streamconverters have code like:
fooStreamConverter::OnDataAvailable {
...
listener->OnStartRequest();
listener->OnDataAvailable();
}
so if the listener expects that it gets no data after a suspend(), this is a
problem.
streamlisteners with this problem:
binhex
unknowndecoder
nstxttohtml
Comment 1•21 years ago
|
||
Could someone please elaborate on why this blocks #69938? This bug seems to be
Windows-only. Would it be possible to fix #69938 without fixing this on Linux, then?
Reporter | ||
Comment 2•21 years ago
|
||
sorry, I tend to forget to fix the os/platform fields
no, this really is a dependency
OS: Windows 2000 → All
Hardware: PC → All
Comment 3•21 years ago
|
||
Thanks for the quick response, Christian.
Comment 4•21 years ago
|
||
yup, this is a well known problem. i thought about ways to fix this for some
time, and unfortunately i never came up with a good solution. see bug 93055,
which is essentially the core bug here.
our APIs suck is what it basically means. you'd like each stream converter to
implement the nsIRequest that it passes to its listener. this would allow the
stream converter to know if the consumer decides not to consume any more data,
but the problem is that that same nsIRequest needs to be QI'able to a variety of
interfaces (e.g., nsIHttpChannel). therein lies the problem.
the async streams work was a step toward solving this problem because it makes
it possible to push all notions of async stream operations (reading, suspending,
and resuming) behind a common interface: nsIAsyncInputStream. however, we still
have the channel + streamlistener API.
ideally, our stream converters should just be streams that read from other
streams. unfortunately though, we have a stream converter interface that has
led to crufty things like nsUnknownDecoder, which does not actually convert
data!! nsMultiMixedConv is another example of a wacky stream converter. stream
converters should just convert the bytes in a stream. these other beasts should
be expressed differently.
can you tell that i hate this bug? ;-)
Depends on: 93055
Comment 5•21 years ago
|
||
one solution might be to introduce an interface like nsIMultiPartChannel (maybe
call it nsILayeredRequest) that can be used by consumers to get at the
underlying nsIRequest (the baseRequest). then you could have stream converters
implement nsILayeredRequest and pass their nsILayeredRequest impl to their
listener. finally, go out and teach every nsIStreamListener implementation
about nsILayeredRequest and celebrate victory :-/
Reporter | ||
Comment 6•21 years ago
|
||
> can you tell that i hate this bug? ;-)
heh :) I hate it too... I couldn't yet come up with a real good solution, though
I did think about it.
> finally, go out and teach every nsIStreamListener implementation
> about nsILayeredRequest and celebrate victory :-/
I don't think that changing how a frozen interface behaves is that good an idea :/
maybe nsIStreamConverter should just have two additional methods (Suspend,
Resume) that are called by the channel when the streamlistener calls
Suspend/Resume on the channel... it would make the converter stop sending
further ODAs. Resume would make it continue with them. (this is basically what
bz suggested to me, some time ago, in private mail iirc). this would make the
whole streamconverting transparent to streamlisteners, which is apparently how
streamconverters were designed. maybe I should say "implemented", not sure they
really were designed.
Comment 7•21 years ago
|
||
the problem is that stream converters are designed to be transparent to the
channels as well :-(
what happens to a drop-in protocol handler? does it have to understand how to
talk to nsIStreamConverter as well? it can't since nsIStreamConverter is not
frozen :-(
Comment 8•21 years ago
|
||
> the problem is that stream converters are designed to be transparent to the
> channels
They can't be transparent to both ends, since sometimes you really do need to
give them some information.... It's an issue in the context of things other
than suspend/resume (eg content-encoding handling).
We have real bugs. We can't solve them for all listener/protocol-impl
combinations because one or the other will need to know about stream converters.
So which is it less painful to change? Unchanged setups will continue with the
bugs, but at least going forward the bugs can be eliminated in newer code...
Comment 9•21 years ago
|
||
yeah, maybe a partial solution is better than no solution :-/
Reporter | ||
Comment 10•21 years ago
|
||
-> default owner, I don't see me fixing this anytime soon
Assignee: cbiesinger → darin
Updated•21 years ago
|
Target Milestone: --- → Future
Updated•18 years ago
|
Assignee: darin → nobody
QA Contact: benc → networking
Updated•9 years ago
|
Whiteboard: [necko-would-take]
Comment 11•9 years ago
|
||
perf key word?
Comment 12•7 years ago
|
||
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: -- → P5
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•