{"id":854,"date":"2011-06-07T14:01:12","date_gmt":"2011-06-07T03:01:12","guid":{"rendered":"http:\/\/blog.mozilla.org\/nnethercote\/?p=854"},"modified":"2011-06-07T14:01:12","modified_gmt":"2011-06-07T03:01:12","slug":"what-is-the-point-of-non-fatal-assertions","status":"publish","type":"post","link":"https:\/\/blog.mozilla.org\/nnethercote\/2011\/06\/07\/what-is-the-point-of-non-fatal-assertions\/","title":{"rendered":"What is the point of non-fatal assertions?"},"content":{"rendered":"

When you run a debug build of Firefox you get a lot of stuff printed to stderr.\u00a0 Some of it looks like this:<\/p>\n

 ++DOCSHELL 0x7f53d836b800 == 3\r\n ++DOMWINDOW == 5 (0x7f53d836c078) [serial = 5] [outer = (nil)]<\/pre>\n
and I imagine it’s occasionally useful.<\/p>\n
You also see a lot of warnings like these, which I just saw (with duplicates removed) when I just started up Firefox, loaded www.mozilla.com, and then exited:<\/p>\n
 WARNING: 1 sort operation has occurred for the SQL statement '0x7f53e3d32208'.  See https:\/\/developer.mozilla.org\/En\/Storage\/Warnings details.: file \/home\/njn\/moz\/mc2\/storage\/src\/mozStoragePrivateHelpers.cpp, line 144\r\n WARNING: GetDefaultCharsetForLocale: need to add multi locale support: file \/home\/njn\/moz\/mc2\/intl\/locale\/src\/unix\/nsUNIXCharset.cpp, line 146\r\n WARNING: Ignoring duplicate observer.: file \/home\/njn\/moz\/mc2\/modules\/libpref\/src\/nsPrefBranch.cpp, line 594\r\n WARNING: not an nsIRDFRemoteDataSource: 'remote != nsnull', file \/home\/njn\/moz\/mc2\/rdf\/datasource\/src\/nsLocalStore.cpp, line 312\r\n WARNING: NS_ENSURE_SUCCESS(rv, 0) failed with result 0x8000FFFF: file \/home\/njn\/moz\/mc2\/content\/base\/src\/nsContentUtils.cpp, line 2527\r\n WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file \/home\/njn\/moz\/mc2\/content\/base\/src\/nsFrameLoader.cpp, line 415\r\n WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8053000C: file \/home\/njn\/moz\/mc2\/content\/base\/src\/nsGenericElement.cpp, line 5484\r\n WARNING: NS_ENSURE_TRUE(aURI) failed: file \/home\/njn\/moz\/mc2\/content\/base\/src\/nsContentUtils.cpp, line 4706\r\n WARNING: NS_ENSURE_TRUE(!(mAsyncExecutionThread)) failed: file \/home\/njn\/moz\/mc2\/storage\/src\/mozStorageConnection.cpp, line 784\r\n WARNING: NS_ENSURE_TRUE(pusher.Push(aBoundElement)) failed: file \/home\/njn\/moz\/mc2\/content\/xbl\/src\/nsXBLProtoImplMethod.cpp, line 327\r\n WARNING: nsExceptionService ignoring thread destruction after shutdown: file \/home\/njn\/moz\/mc2\/xpcom\/base\/nsExceptionService.cpp, line 197\r\n WARNING: SQLite returned error code 1 , Storage will convert it to NS_ERROR_FAILURE: file \/home\/njn\/moz\/mc2\/storage\/src\/mozStoragePrivateHelpers.cpp, line 113\r\n WARNING: Subdocument container has no content: file \/home\/njn\/moz\/mc2\/layout\/base\/nsDocumentViewer.cpp, line 2398\r\n WARNING: Subdocument container has no frame: file \/home\/njn\/moz\/mc2\/layout\/base\/nsDocumentViewer.cpp, line 2418\r\n WARNING: Subdocument container has no frame: file \/home\/njn\/moz\/mc2\/layout\/base\/nsDocumentViewer.cpp, line 2418<\/pre>\n
The NS_ENSURE_SUCCESS<\/code> and NS_ENSURE_TRUE<\/code> ones in particular look like assertion failures.\u00a0 I’ve heard before that Firefox assertions (outside the JS engine) are non-fatal.\u00a0 I was wondering about this last week and had planned to ask someone about it in more detail.<\/p>\n
But before I did that, I spent several hours yesterday debugging the crash in bug 654573<\/a>.\u00a0 Turns out the problem is that this assertion in mozStorageConnection.cpp fails:<\/p>\n
 NS_ENSURE_FALSE(mAsyncExecutionThread, NS_ERROR_UNEXPECTED);<\/pre>\n
This causes this warning to be printed out at run-time:<\/p>\n
 WARNING: NS_ENSURE_TRUE(!(mAsyncExecutionThread)) failed: file \/home\/njn\/moz\/mc1\/storage\/src\/mozStorageConnection.cpp, line 799<\/pre>\n
Oh, and just to make things really complicated, here’s the definition of NS_ENSURE_FALSE, from nsDebug.h:<\/p>\n
 #define NS_ENSURE_TRUE(x, ret)                                \\\r\n   PR_BEGIN_MACRO                                              \\\r\n     if (NS_UNLIKELY(!(x))) {                                  \\\r\n        NS_WARNING(\"NS_ENSURE_TRUE(\" #x \") failed\");           \\\r\n        return ret;                                            \\\r\n     }                                                         \\\r\n   PR_END_MACRO\r\n #define NS_ENSURE_FALSE(x, ret)\r\n   NS_ENSURE_TRUE(!(x), ret)<\/pre>\n
Oh look;\u00a0 if the condition fails it not only prints out a warning message, it also does an early return.\u00a0 And that’s exactly what was causing the crash, because some code that followed the assertion wasn’t run, which meant that a database connection wasn’t closed properly, which caused a use-after-free error.<\/p>\n
If this assertion had been fatal, I wouldn’t have spent several hours yesterday debugging this crash, because there wouldn’t have been a crash;\u00a0 there would have been an assertion failure, and the problem would have been immediately obvious.\u00a0 (As it happens, I eventually worked out the problem when I noticed that warning message among the pages of output that Firefox produces.)\u00a0 If the assertion had been non-fatal but did not have the early return, there also probably wouldn’t have been a crash, but it’s hard to know, because once an assertion fails you’re pretty much in no-man’s land.<\/p>\n
So, in summary, here are my thoughts about assertions:<\/p>\n
    \n
  • Non-fatal assertions are close to useless.\u00a0 They don’t get fixed.\u00a0 The warnings just get lost among the noise of all the messages that go to stderr.<\/li>\n
  • Non-fatal assertions that return early are worse than useless.\u00a0 Especially since the early-return behaviour is entirely non-obvious from their names.<\/li>\n<\/ul>\n
    One suggestion I heard on IRC yesterday was that there just aren’t  enough developers in general to fix all the assertion failures if we were to make  them fatal.\u00a0 But in contrast, the JS engine is able to use fatal assertions  because it has a higher developer-to-code ratio than the rest of  Firefox.\u00a0 (This makes me also think of compiler warnings, where there’s a similar disparity between the JS engine and the rest of Firefox.)<\/p>\n
    Because this is Mozilla, I’m sure there is a ton of history behind this current situation that I’m unaware of.\u00a0 I’d love to know what that is, and why we let obvious defects — because an assertion failure unambiguously indicates there’s a defect, be it in the code or the assertion itself — remain.\u00a0 Thanks!<\/p>\n
     <\/p>\n","protected":false},"excerpt":{"rendered":"
    When you run a debug build of Firefox you get a lot of stuff printed to stderr.\u00a0 Some of it looks like this: ++DOCSHELL 0x7f53d836b800 == 3 ++DOMWINDOW == 5 (0x7f53d836c078) [serial = 5] [outer = (nil)] and I imagine it’s occasionally useful. You also see a lot of warnings like these, which I just […]<\/p>\n","protected":false},"author":139,"featured_media":0,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[528,30],"tags":[],"_links":{"self":[{"href":"https:\/\/blog.mozilla.org\/nnethercote\/wp-json\/wp\/v2\/posts\/854"}],"collection":[{"href":"https:\/\/blog.mozilla.org\/nnethercote\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/blog.mozilla.org\/nnethercote\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/blog.mozilla.org\/nnethercote\/wp-json\/wp\/v2\/users\/139"}],"replies":[{"embeddable":true,"href":"https:\/\/blog.mozilla.org\/nnethercote\/wp-json\/wp\/v2\/comments?post=854"}],"version-history":[{"count":0,"href":"https:\/\/blog.mozilla.org\/nnethercote\/wp-json\/wp\/v2\/posts\/854\/revisions"}],"wp:attachment":[{"href":"https:\/\/blog.mozilla.org\/nnethercote\/wp-json\/wp\/v2\/media?parent=854"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/blog.mozilla.org\/nnethercote\/wp-json\/wp\/v2\/categories?post=854"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/blog.mozilla.org\/nnethercote\/wp-json\/wp\/v2\/tags?post=854"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}