mork assertions in history code when clicking on certain urls



MailNews Core
17 years ago
10 years ago


(Reporter: dmose, Assigned: Bienvenu)


Firefox Tracking Flags

(Not tracked)



(3 attachments)



17 years ago
The subject pretty much sez it all, I think.  Filesystem corruption is a fact of
life, and Mozilla should be more robust in the face of it.

Comment 1

17 years ago
AFAIK, we're plenty robust in the fact of corrupt .msf files - we don't crash,
we delete them and regenerate the data. Aren't you really complaining about the
annoying asserts when running a debug build? I suppose I could turn off the
asserts, but for the record, I never see these asserts, and my db's never get
corrupted. Are you running two clients against the same profile?
I can consistently get a corrupted database using a testcase to bug 73958.

The testcase for that bug consists of a XUL file that I open as if it was normal
file. The XUL file includes a JS file. In onload handler we set timeout function
which tries to do"GET","some XML file URL").

Comment 3

17 years ago
I believe that's a corrupt history file of some sort - I don't know what's
getting poked there but I can look into it (though handling of the corrupt
database is up to the mork client)

Comment 4

17 years ago
well, what global history will do is try to open the file, and if it fails in
any way, then it blows away global history, and starts with a fresh one... so I
believe that nsGlobalHistory is doing the right thing

But I have to say, I don't see the harm in a few asserts if the file is corrupt.
Disk corruption is VERY rare, you (dmose) probably corrupted your history db by
running two instances of mozilla at the same time, which just isn't supported

Since the global history component does the right thing with mork DBs when they
are corrupt, I would say that the code is working as designed :)

Comment 5

17 years ago
I was really proxy-filing this bug for Heikki, so it's really his call, but it
sounds like _something_ wasn't do the right thing with a corrupt db.  Heikki?

Comment 6

17 years ago
The problem Heikki is seeing is a real problem, and should be fixed, but not by
removing the asserts.

Comment 7

17 years ago
Right.  I didn't mean to imply with the summary that the asserts were bad;
merely that they can't be a substitute for real error handling.
Yeah, removing the asserts is not the right thing if you are supposed to not hit
them regularly (i.e. corrupted db -> cleanup -> no more assert). But since I
seem to consistently get the assert (and more asserts as time gooes by) it seems
like things aren't cleared as they should, or something else funky is going on.

I had had one or two asserts for a long time, then suddenly started gettings
dozens of asserts on first document load. Once I deleted history.dat the asserts
went away, but as soon as I opened my testcase I got one assert which I see on
first document load (now I am back to something like 2 or three asserts on the
first document load).

Comment 9

17 years ago
I'm changing the summary - I've found the cause of this - someone is passing
history a null string, and history in turn is trying to set a mork column to the
null string, which mork doesn't like. I'll attach a fix.
Summary: corrupt mork files should be handled gracefully, not with asserts → mork assertions in history code when clicking on certain urls

Comment 10

17 years ago
Created attachment 37222 [details] [diff] [review]
proposed fix for one cause of assertion

Comment 11

17 years ago
Does this fix the assertion you were seeing on the test case, Heikki? I don't
know what would cause more and more assertions to appear, but I can fix the
simple case of trying to load a file url causing an assertion.
Hmm... It still does not fix it for me. I'll attach my testcases.

Btw, you might want to replace the cast with .get() in the patch.
Created attachment 37225 [details]
tupshin.xul (save to local file system)
Created attachment 37226 [details]
tupshin.js (save to local file system)
Ok, save the two attachments to local disc. Then start up your browser with URL
to tupshin.xul as the command line argument. You should see some debug dumps in
the console. Quit the application. Restart (same default arg). This time you
should get the mork assert.

Comment 16

17 years ago
cool. sr=alecf if you indent the line below the if()

Comment 17

17 years ago
oh, just saw that it doesn't fix it. 
david, you could just try fixing SetRowValue() for both PRUnichar* and char*

Comment 18

17 years ago
So, Alec, I'm reluctant to change SetRow to simply ignore callers who pass the
null string because someone might want to empty out a column (i.e., set it to ""
when it used to be some other value). I could convert null to "", but then we'd
have the empty columns in the db (unless mork is smart enough to not write those
columns out - that's possible, actually). So, I was trying to fix the caller.

Heikki, exactly what should the command line arguments be? I saved tushpin.xul
and js to c:\tmp. should the command line be just c:\tmp\tushpin.xul, or
file://c:/tmp/tushpin.xul? I'll try the former.

Comment 19

17 years ago
Heikki, I tried mozilla c:\tmp\tupshin.xul, and I didn't see any asserts, either
on the first run, or the second run. I haven't tried this without my patch, but
I can do so.
I have used the file URL argument. I think you actually need (or at least you
needed that at some point) to have 3 slashes there...


I did not see the assertion on first launch, only on the second and subsequent
launches. But if you do not see the assertions with this testcase then I am
pretty much out of ideas...

Comment 21

17 years ago
*** Bug 84329 has been marked as a duplicate of this bug. ***

Comment 22

17 years ago
I think Alec fixed this - I no longer see the assert, anyway.
Last Resolved: 17 years ago
Resolution: --- → WORKSFORME
QA Contact: esther → stephend
### XPCOM_MEM_LEAK_LOG defined -- logging leaks to c:\WINNT\Temp\leaklog.txt
Type Manifest File: C:\moz_src\mozilla\dist\WIN32_D.OBJ\bin\components\xpti.dat
nsNativeComponentLoader: autoregistering begins.
nsNativeComponentLoader: autoregistering succeeded
nNCL: registering deferred (0)
WARNING: CSSLoaderImpl::LoadAgentSheet: Load of URL 'file:///C:/Documents%20and%
 file c:\moz_src\mozilla\content\html\style\src\nsCSSLoader.cpp, line 1611
WARNING: CSSLoaderImpl::LoadAgentSheet: Load of URL 'file:///C:/Documents%20and%
, file c:\moz_src\mozilla\content\html\style\src\nsCSSLoader.cpp, line 1611
WARNING: unable to load UA style sheet, file 
Note: verifyreflow is disabled
WARNING: unable to load UA style sheet, file 
Note: styleverifytree is disabled
Note: frameverifytree is disabled
JavaScript error:
chrome://wallet/content/walletOverlay.js line 1: redeclaration of const hide

JavaScript error:
chrome://wallet/content/walletOverlay.js line 1: redeclaration of const hide

JavaScript error:
 line 0: myhttprequest is not defined

Document file:///C%3A%5CWINNT%5Ctemp%5Ctupshin.xul loaded successfully
###!!! ASSERTION: cannot handle open-ended tag name query: 'Error', file 
Start reading in bookmarks.html
Finished reading in bookmarks.html  (47000 microseconds)
WARNING: requested removal of nonexistent window
, file 
line 846
### nsCacheProfilePrefObserver::Observe [topic=xpcom-shutdown data=]
nsPluginHostImpl::Observe "xpcom-shutdown"

That's everything I see until all the annoying xpcom shutdown notices ;-)

This is on Windows 2000, debug build, fresh trunk.

I do not see any mork assertions whatsoever.

Verified worksforme.
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.