crash [@ nsMsgFilterService::BackUpFilterFile(nsILocalFile*, nsIMsgWindow*)] bp-a92775cc-20d2-409a-9705-f55642101101 Frame Module Signature [Expand] Source 0 thunderbird-bin nsMsgFilterService::BackUpFilterFile mailnews/base/search/src/nsMsgFilterService.cpp:195 1 thunderbird-bin nsMsgFilterService::OpenFilterList mailnews/base/search/src/nsMsgFilterService.cpp:138 2 thunderbird-bin nsMsgIncomingServer::GetFilterList mailnews/base/util/nsMsgIncomingServer.cpp:1120 3 thunderbird-bin nsMsgDBFolder::GetFilterList mailnews/base/util/nsMsgDBFolder.cpp:4908 4 thunderbird-bin nsImapMailFolder::UpdateFolderWithListener mailnews/imap/src/nsImapMailFolder.cpp:697 5 thunderbird-bin nsImapMailFolder::UpdateFolder mailnews/imap/src/nsImapMailFolder.cpp:678 6 thunderbird-bin nsImapMailFolder::GetNewMessages mailnews/imap/src/nsImapMailFolder.cpp:2619
The crash is no longer accessible? Is it your crash? Can you reproduce it?
May be related to bug 537026 and bug 612063. The line nsMsgFilterService::OpenFilterList mailnews/base/search/src/nsMsgFilterService.cpp:138 is also there. May be a problem in the BackUpFilterFile function, in all three bugs.
(In reply to :aceman from comment #1) > The crash is no longer accessible? apparently yes. > Is it your crash? Can you reproduce it? not my crash - mine almost always have my email address and/or comment. it is a rare crash, but still exists. here are two recents with comments bp-ad2a1223-8f86-45f8-8aeb-562ef2120103 bp-53797f74-e4bb-4d41-9149-172012120102 (In reply to :aceman from comment #2) > May be related to bug 537026 and bug 612063. The line > nsMsgFilterService::OpenFilterList > mailnews/base/search/src/nsMsgFilterService.cpp:138 is also there. May be a > problem in the BackUpFilterFile function, in all three bugs. indeed - pick one and stab it inthe heart! :)
I could probably add some null checks at least, but how to test if it helps? :)
The reports still continue (like 6af35d1b-2943-4a50-b07e-3a87a2120829) so let's try some patch.
Created attachment 656910 [details] [diff] [review] patch
Comment on attachment 656910 [details] [diff] [review] patch In general, since my feedback has been requested, let me give the feedback that I do not agree that it is worthwhile to go to existing code and make cosmetic changes simply to bring the code up to current standards. It makes work for everyone, introduces the possibility of errors, plus it complicates the code history when you are tracing back problems. So I would not normally approve such changes, unless they are in the context of an extensive rework of existing code. But my opinions may not reflect standard policy. Back to the crash itself, the only substantial change here is the error check: NS_ENSURE_ARG_POINTER(aFilterFile); That is a correct thing to do, and is a non-cosmetic change that I would typically approve. But as in my comments on the patch for bug 537026, I cannot see a theory of the crash that this would solve. aFilterFile has already been confirmed as non-null in the parent to this call. AFAIK, this type of pointer does not suddenly get reset to null (unlike the message summary database pointer, which is a major design flaw in our code). Still, this crash is occurring on that exact line that would hint at a null aFilterFile, and this change would not hurt. So what I would approve of is a single line change to add this check, but I do not expect it will solve the crash. I agree with earlier comments that the root cause if probably the same here as in bug 537026, but I have no theories what that root cause might be.
Comment on attachment 656910 [details] [diff] [review] patch I think I'm with Kent, the NS_ENSURE_ARG_POINTER isn't really necessary as the caller has already used this object. The only thing I can think is that something is slightly messed somewhere, as I can't see anything wrong with the way aFilterFile is used prior to being passed to this function. I'm pretty sure this wouldn't fix the crash though. The other changes, I'd perhaps accept if we were doing stuff in the file or cleanup or something, but not on their own.
Thanks, no problem.
signature has changed to nsMsgFilterService::BackUpFilterFile(nsIFile*, nsIMsgWindow*)
and fwiw, it's rare so no great need to pursue this as far as I can tell.
this year no crashes newer than version 15. So either gone, or not actionable without steps https://crash-stats.mozilla.com/signature/?date=%3E2016-02-01&signature=nsMsgFilterService%3A%3ABackUpFilterFile&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_sort=-date&page=1