Open Bug 227113 Opened 21 years ago Updated 2 years ago

Streamconverters don't deal well with Suspend/Resume

Categories

(Core :: Networking, defect, P5)

defect

Tracking

()

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
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?
sorry, I tend to forget to fix the os/platform fields no, this really is a dependency
OS: Windows 2000 → All
Hardware: PC → All
Thanks for the quick response, Christian.
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
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 :-/
> 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.
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 :-(
> 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...
yeah, maybe a partial solution is better than no solution :-/
-> default owner, I don't see me fixing this anytime soon
Assignee: cbiesinger → darin
Target Milestone: --- → Future
No longer blocks: 57342
Assignee: darin → nobody
QA Contact: benc → networking
Whiteboard: [necko-would-take]
perf key word?
Priority: -- → P5
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.