Closed Bug 10456 Opened 25 years ago Closed 25 years ago

[DOGFOOD] [BLOCKER] External entity loading should be asynchronous

Categories

(Core :: Networking, defect, P1)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: morse, Assigned: nisheeth_mozilla)

References

Details

(Whiteboard: Blocking all valid XML parsing; blocking QA of XHTML support)

Select menu item edit/wallet/wallet-contents.  Get a crash with the following
stack trace.

NTDLL! 77f76274()
nsDebug::Error(const char * 0x0a270ab8, const char * 0x0a270a80, int 150) line
195 + 13 bytes
nsHTTPChannel::OpenInputStream(nsHTTPChannel * const 0x0ac27090, unsigned int 0,
int -1, nsIInputStream * * 0x0012be24) line 150 + 21 bytes
wallet_FetchFromNetCenter(char * 0x0ad438e4, char * 0x0ad438d4) line 1340 + 20
bytes
wallet_FetchFieldSchemaFromNetCenter() line 1390 + 15 bytes
wallet_Initialize() line 1624
WLLT_PreEdit(nsAutoString & {...}) line 2175
nsWalletlibService::WALLET_PreEdit(nsWalletlibService * const 0x0ac2f540,
nsAutoString & {...}) line 99 + 9 bytes
WalletEditorImpl::GetValue(WalletEditorImpl * const 0x0ab30f50, char * *
0x0012c060) line 94 + 16 bytes
XPTC_InvokeByIndex(nsISupports * 0x0ab30f50, unsigned int 4, unsigned int 1,
nsXPTCVariant * 0x0012c060) line 135
nsXPCWrappedNativeClass::CallWrappedMethod(JSContext * 0x0a22d550,
nsXPCWrappedNative * 0x0ab30270, const XPCNativeMemberDescriptor * 0x0ab302e8,
nsXPCWrappedNativeClass::CallMode CALL_METHOD, unsigned int 0, long *
0x00ef6cc0, long * 0x0012c268) line 511 + 44 bytes
WrappedNative_CallMethod(JSContext * 0x0a22d550, JSObject * 0x00ef41d8, unsigned
int 0, long * 0x00ef6cc0, long * 0x0012c268) line 130
js_Invoke(JSContext * 0x0a22d550, unsigned int 0, unsigned int 0) line 654 + 26
bytes
js_Interpret(JSContext * 0x0a22d550, long * 0x0012ca90) line 2228 + 15 bytes
js_Invoke(JSContext * 0x0a22d550, unsigned int 0, unsigned int 0) line 670 + 13
bytes
js_Interpret(JSContext * 0x0a22d550, long * 0x0012d274) line 2228 + 15 bytes
js_Invoke(JSContext * 0x0a22d550, unsigned int 0, unsigned int 0) line 670 + 13
bytes
js_Interpret(JSContext * 0x0a22d550, long * 0x0012da58) line 2228 + 15 bytes
js_Invoke(JSContext * 0x0a22d550, unsigned int 0, unsigned int 0) line 670 + 13
bytes
js_Interpret(JSContext * 0x0a22d550, long * 0x0012e23c) line 2228 + 15 bytes
js_Invoke(JSContext * 0x0a22d550, unsigned int 1, unsigned int 2) line 670 + 13
bytes
js_InternalCall(JSContext * 0x0a22d550, JSObject * 0x00e85798, long 15680024,
unsigned int 1, long * 0x0012e37c, long * 0x0012e384) line 747 + 15 bytes
JS_CallFunctionValue(JSContext * 0x0a22d550, JSObject * 0x00e85798, long
15680024, unsigned int 1, long * 0x0012e37c, long * 0x0012e384) line 2643 + 29
bytes
nsJSEventListener::HandleEvent(nsIDOMEvent * 0x0ac285a0) line 97 + 34 bytes
nsEventListenerManager::HandleEvent(nsIPresContext & {...}, nsEvent *
0x0012e538, nsIDOMEvent * * 0x0012e4a0, unsigned int 3, nsEventStatus &
nsEventStatus_eIgnore) line 959 + 21 bytes
GlobalWindowImpl::HandleDOMEvent(GlobalWindowImpl * const 0x0a22ca24,
nsIPresContext & {...}, nsEvent * 0x0012e538, nsIDOMEvent * * 0x0012e4a0,
unsigned int 1, nsEventStatus & nsEventStatus_eIgnore) line 2744
nsWebShell::OnEndDocumentLoad(nsWebShell * const 0x0a247e54, nsIDocumentLoader *
0x0a246030, nsIChannel * 0x0a171590, int 0, nsIDocumentLoaderObserver *
0x0a247e54) line 2963 + 34 bytes
nsDocLoaderImpl::FireOnEndDocumentLoad(nsIDocumentLoader * 0x0a246030, int 0)
line 1100
nsDocLoaderImpl::OnStopRequest(nsDocLoaderImpl * const 0x0a246034, nsIChannel *
0x0a171590, nsISupports * 0x00000000, unsigned int 0, const unsigned short *
0x00000000) line 1025
nsOnStopRequestEvent::HandleEvent(nsOnStopRequestEvent * const 0x0ac28490) line
284
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x0ac28494) line 149 + 12 bytes
PL_HandleEvent(PLEvent * 0x0ac28494) line 509 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00a60360) line 470 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x083504a6, unsigned int 49404, unsigned int 0,
long 10879840) line 932 + 9 bytes
USER32! 77e71268()
00a60360()
Assignee: gagan → rpotts
Target Milestone: M9
I assume this happens with the Necko build only?  What builds - trunk and branch
was this tested on?
Severity: normal → blocker
This bug is a blocker right now because it prevents wallet from being used.
Raising severity.

BTW, you won't observe the problem with builds made last night or today.  In my
own private tree I have temporarily bypassed the call to
wallet_FetchFromNetcenter and then accidentally checked in my file containing
that change.  As soon as the tree opens I will check in a new version without
the change.

In answer to Jan's question, this is happening with Necko build only.  It is
being tested on builds made from the tip but by using NECKO=1 in the build.
Priority: P3 → P1
Summary: Error on OpenInputStream → [BLOCK] Error on OpenInputStream
Gagan, does http support sync IO yet?
no it doesn't as yet.
*** Bug 10782 has been marked as a duplicate of this bug. ***
*** Bug 10786 has been marked as a duplicate of this bug. ***
*** Bug 10783 has been marked as a duplicate of this bug. ***
Can someone give me an estimate as to when this bug will get fixed?  It is a
serious bug that is preventing any useage or testing of wallet.  Yet the bug is
still marked as "new" although it was reported five days ago.
*** Bug 10526 has been marked as a duplicate of this bug. ***
This bug appears to be related to bug 11016 although I'm not prepared to say at
this time that the two bugs are duplicates.  I'll leave that up to the necko
team to decide.
This bug may appear to be fixed, but it is not.  I just checked in a work-around
that is avoiding the crash but it is full of memory leaks and it does not work
on the mac.

For the purpose of testing this bug, go to the file
extensions/wallet/src/wallet.cpp and comment out the #define NEWFETCH line.
That will remove my work-around and make the bug reappear.
No sync in HTTP as yet.
Summary: [BLOCK] Error on OpenInputStream → Error on OpenInputStream for http channel
Changing title since this is no longer a blocker due to workaround.

Steve, please report any leaks you've found separately. Thanks.
The leaks I was referring to are not in necko but rather in the temporary patch
that Andreas provided.
*** Bug 11340 has been marked as a duplicate of this bug. ***
*** Bug 11356 has been marked as a duplicate of this bug. ***
*** Bug 11356 has been marked as a duplicate of this bug. ***
Summary: Error on OpenInputStream for http channel → Implement OpenInputStream for http channel
Is this still an M9 blocker? Or can it be moved to an M10 feature...
What do define as a blocker?  It blocked one of the wallet features (downloadig
of mapping tables from server) and, as a consequence, I had to disable that
feature from wallet for M9.
And hence, this can be moved off to M10. Right ?
Blocks: 7530
Target Milestone: M9 → M10
Sounds good for move to M10.  paulmac, please make sure to release note that this
feature is disabled.
*** Bug 10703 has been marked as a duplicate of this bug. ***
Wallet shouldn't depend on this right. Even if necko fixed this, we decided that
wallet will implement async update of the file instead of sync. So I am removing
this from any wallet list.

Steve correct me if I am wrong.
*** Bug 11948 has been marked as a duplicate of this bug. ***
Severity: blocker → critical
m11
Target Milestone: M10 → M11
Blocks: 15027
No longer blocks: 15027
Whiteboard: Blocking QA of XHTML support
This is blocking one of my projects, as parsing of valid XHTML files is hosed
until this bug can be fixed.
Blocks: 15027
[Oops, accidentally nuked dependencies...]
I don't know anything about xhtml, but for it to use OpenInputStream, it must be
running on some other thread besides the main, mozilla, thread. Is that the
case?
Severity: critical → blocker
Summary: Implement OpenInputStream for http channel → [BLOCKER] Implement OpenInputStream for http channel
Whiteboard: Blocking QA of XHTML support → Blocking all valid XML parsing; blocking QA of XHTML support
The problem is the XML parser needs a blocking read to parse the DTD of the
document. All valid XML documents have DTDs. Therefore, it is currently TOTALLY
IMPOSSIBLE to view valid XML documents on the web, as blocking reads do not work
by http.

In bug 11948, nisheeth wrote:
: [For valid XML documents, the XML parser] attempts to access a DTD file on a
: remote host.  This is currently not working because Necko does not support
: synchronous file loading. Once Rick Potts fixes bug 10456, this will [work].

I'm increasing the priority to blocker. This is a major beta blocker since XML
parsing is supposed to be one of our main "selling" points!

If the problem is in the XML parser implementation (i.e, XML shouldn't be
using this call to get the DTD), then tell nisheeth...
Why is the XML handling of DTD files any different than that of HTML for linked
scripts and style sheets?

In HTML, these linked files are downloaded asynchronously (but with the parser
blocked) to allow UI feedback.  If the XML parser did a blocking read on the UI
thread *all user feedback would stop*.

Nisheeth, can XML handle its DTD files the same way that <SCRIPT SRC=foo.js> is
done by HTML?  I think that this is the right solution...
Assignee: rpotts → nisheeth
I wasn't on the cc list.  Sorry for not responding to your question earlier,
Rick.

I agree with Rick that DTD loading should be asynchronous.  The decision to make
it synchronous was made by the intl folk who implemented the external DTD load
feature; they probably thought that this was OK because their DTD files were
always resident on the local disk.

I'm putting this on my plate for M11...
*** Bug 15027 has been marked as a duplicate of this bug. ***
Status: NEW → ASSIGNED
Summary: [BLOCKER] Implement OpenInputStream for http channel → [BLOCKER] External entity loading should be asynchronous
Updating summary to reflect the true nature of what we are blocked on.
Blocks: 16950
Summary: [BLOCKER] External entity loading should be asynchronous → [DOGFOOD] [BLOCKER] External entity loading should be asynchronous
Please explain how this is blocking XML parsing.  We will look at in PDT daily
mtg tomorrow to decide PDT+ or PDT- based on new comments.
This prevents any XML file with a DOCTYPE with an http URL (such as the ones for
XHTML) from loading.  That is, any *valid* XHTML file won't load.

I modified all my DOM test XML files by removing the DOCTYPE, but not everyone
will do that...
I have changes in my tree that fix this.  We are only going to load external
DTDs when the DTD is referred to with a chrome url.  This is because external
DTD loading is an "internal" feature that we need for doing entity substitution
in mozilla.  We do not want to expose external DTD loading as a general XML
feature because doing so takes our XML parser a "not quite validating" parser
and we end up with a non-standards-compliant XML implementation.  We'd rather
restrict mozilla's XML parser to be non-validating so that we remain standards
compliant.
Actually, I should say that my changes will fix the "XHTML page not loading
bugs" that are related to this bug.  The problem that DTD loading is synchronous
remains.  But, for now, that problem is lower priority because chrome urls will
map to file urls which do support synchronous access.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
The fix is checked in.  Marking as such.
I've opened up bug 17236 on myself to track making chrome url'd DTD loads
asynchronous.
Blocks: 15270
Status: RESOLVED → VERIFIED
verified with pages from http://www.xhtml.org using 10/29 build
Bulk move of all Necko (to be deleted component) bugs to new Networking

component.
You need to log in before you can comment on or make changes to this bug.