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.
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 XMLHttpRequest.open("GET","some XML file URL").
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)
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 :)
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?
The problem Heikki is seeing is a real problem, and should be fixed, but not by removing the asserts.
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).
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.
Status: NEW → ASSIGNED
Summary: corrupt mork files should be handled gracefully, not with asserts → mork assertions in history code when clicking on certain urls
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.
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.
cool. sr=alecf if you indent the line below the if()
oh, just saw that it doesn't fix it. david, you could just try fixing SetRowValue() for both PRUnichar* and char*
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.
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... file:///c:/tmp/tushpin.xul 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...
*** Bug 84329 has been marked as a duplicate of this bug. ***
I think Alec fixed this - I no longer see the assert, anyway.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → WORKSFORME
QA Contact: esther → stephend
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.