Closed Bug 1354007 Opened 7 years ago Closed 7 years ago

HTTP and HTTPS links don't open after updating Thunderbird from 45 to 52 version (broken "places.sqlite*" files?)

Categories

(Thunderbird :: Message Reader UI, defect)

52 Branch
defect
Not set
major

Tracking

(thunderbird_esr45 unaffected, thunderbird_esr52+ fixed, thunderbird53 fixed, thunderbird54 fixed, thunderbird55 fixed)

RESOLVED FIXED
Thunderbird 55.0
Tracking Status
thunderbird_esr45 --- unaffected
thunderbird_esr52 + fixed
thunderbird53 --- fixed
thunderbird54 --- fixed
thunderbird55 --- fixed

People

(Reporter: ephemeris.lappis, Assigned: jorgk-bmo)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36

Steps to reproduce:

After update to V52.0 yesterday, no HTTP link opens in the web browser. Before, no problem. I've tried to change default browser from Chrome to Firefox and back, but nothing !


Actual results:

No HTTP link works.


Expected results:

Links to HTTP URL must open in the web browser.
Confirmed on my System :(

I wonder how such a HUGE bug can slip through beta testing?!
Please provide information about the operating system. "Huge" bugs can slip because no-one else can reproduce them.
My OS is Windows 7 (version 6.1.7601).
Chrome (defalut browser) is the last version 57.0.2987.133 (64-bit)
I hope this can help.
Regards.
Windows 8.1 Pro x64 (German) with all Updates. Thunderbird 52 (tested in normal mode and safe-mode). Kaspersky Internet Security 2017. What else could be relevant?

I just tested with a new profile, and its working there. Where in the prefs.ini (or elsewhere) can I look for the error in the old profile? It would be hell of a work to rebuild the old profile from scratch :/

And sorry for the "huge". I didn't want to offend anybody!
Ahh, there is something logged in the Error-Console ;)

~~~
NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [mozIAsyncHistory.updatePlaces]  contentAreaClick.js:179
	openLinkExternally chrome://communicator/content/contentAreaClick.js:179:3
	contentAreaClick chrome://communicator/content/contentAreaClick.js:163:3
	onclick chrome://messenger/content/messenger.xul:1:8
~~~
(In reply to Claus Berghammer from comment #1)
> I wonder how such a HUGE bug can slip through beta testing?!
There is no bug, and nothing slipped through beta testing.

Have you tried with add-ons disabled? See Help menu.
As I have written before, I tested it in normal-mode and SAFE-MODE (where alle extensions are disabled).
Sorry, I missed that. If it works on a new profile, then TB is not really at fault.

Looking at the code that failed:
NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [mozIAsyncHistory.updatePlaces]  contentAreaClick.js:179

That code reads:

function openLinkExternally(url)
{
  let uri = url;
  if (!(uri instanceof Components.interfaces.nsIURI))
    uri = Services.io.newURI(url, null, null);

  PlacesUtils.asyncHistory.updatePlaces({ <=== That's line 179

So TB is trying to open the link externally in your browser and something doesn't work when updating the history of the links you visited. So try
1) Tools > Clear Recent History
2) remove places.sqlite from your TB profile, which you can open via
   Help > Troubleshooting Information.
Yeahaaa :)
Deleting all "places.sqlite*" files did solve the problem :)
Wiping History (which I tried before) had no effect...

Nevertheless, with 45.8 links worked fine, so somehow was the problem triggered by the upgrade to v52.

I tested two more PCs now:
- On a customer Notebook TB had the same problems, and this customer ha only one extension installed and uses TB not very often.
- On my Fathers PC the upgrade worked. He has ZERO extensions installed ;)

So, for me it happened on 2 from 3 upgrades, that the places database was broken after the upgrade...
@Ephemeris Lappis:
Do you have the extension "Contact Photos" installed on your TB?

My customer and me both has this extension installed, maybe this is causing the problem?
Thanks for letting us know, I'll keep an eye out for it. I'm not aware of any changes to the format of places.sqlite, so I'm a little surprised by this.
Hello !

It finally works !

I've tried to clear the recent history, but it does nothing.

Looking at the profile's folder, I've seen a file named "places.sqlite.corrupt" that can probably explain all the problems. I've deleted all the "places.sqlite*" (places.sqlite, places.sqlite.corrupt, places.sqlite-shm, places.sqlite-wal) and my TB opens now web links as expected.


I think something goes wrong when updating to V52, and not just for me...

Thanks for your help.

Regards.
more examples: bug 1354210, bug 1354149 - unclear whether symptoms are the same
@Ephemeris Lappis:
Do you have the extension "Contact Photos" installed on your TB?

My customer and me both has this extension installed, maybe this is causing the problem?
(In reply to Claus Berghammer from comment #14)
> @Ephemeris Lappis:
> Do you have the extension "Contact Photos" installed on your TB?
> 
> My customer and me both has this extension installed, maybe this is causing
> the problem?

No, I've not installed this extension. My installed extensions are :

CompactHeader	2.1.0	true	{58D4392A-842E-11DE-B51A-C7B855D89593}
Google Search for Thunderbird	1.0	true	gsearch@standard8.plus.com
Lightning	5.4	true	{e2fda1a4-762b-4020-b5ad-a41df1933103}
MM3-ProxySwitch	2016	false	ProxySwitch@MM3Tools.com

And the proxy extension that I don't use anymore is deactivated.

Does it help ?
> No, I've not installed this extension. My installed extensions are :
> 
> Does it help ?

Thanks for your reply. It just would have been interesting if you had the "Contact Photos" extension installed, because it would have been something that all PCs where the upgrade failed would have had in common. Since you don't have it, it must be something else that breaks the database. Or maybe the database was already broken before, but tb45 doesn't use it when opening links?
Same thing happened to me (links not working after update to TB 52.0). Removing all places.sqlite* helped, thanks. 

User-Agent 	Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.0
Build-ID  	20170321161200

Installed extensions:
Lightning	5.4	true	{e2fda1a4-762b-4020-b5ad-a41df1933103}
MinimizeToTray revived (MinTrayR)	1.3.2	true	mintrayr@tn123.ath.cx
Provider fĂĽr Google Kalender	3.1	true	{a62ef8ec-5fdc-40c2-873c-223b8a6925cc}
QR-Code Add-on fĂĽr Lightning	1.2.5	true	lightning-qrcode@harald.judt
Stylish	2.0.7	true	{46551EC9-40F0-4e47-8E18-8E5CF550CFB8}


Additionally I had two random crashes after TB was running for a long time. Those didn't occur before the update, either. Maybe they were related to this problem (report: bp-e9809d03-4688-49b5-9594-9ecd22170406 - yes, I deleted some old, unused addons since then while trying to fix the link issue). If TB crashes again, even after removing places.sqlite, I will report back (in a new report.
Summary: HTTP links don't open → HTTP and HTTPS links don't open after updating Thunderbird from 45 to 52 version (broken "places.sqlite*" files?)
Macro, you're a storage peer, right? We're seeing many problems where people upgrade from TB 45.x to TB 52 and links don't open any more due to some issues with places.sqlite*. See comment #8 for some details. I've upgraded a friend of mine and he had no problem.

Has anything changed that we should be aware of?
Flags: needinfo?(mak77)
:Virtual, nice triaging here, thanks. However, I don't understand which "regression" we want to find here? Surely there is not reproducible error that was regressed by a change elsewhere. All we know is that places.sqlite gets corrupted.

places.slqite is an SQLite database which uses secondary files, places.sqlite-shm, places.sqlite-wal, while the application runs. It should clean up those files when the application closes, but doesn't always do that (bug 686237). You can use the utility "sqlite3.exe places.sqlite vacuum" to force an internal clean-up of the database. I do this before backing up places.sqlite from my FF profile.

My assumption is this: For some users the upgrade happened where the secondary files existed. Maybe I'm wrong, only a storage person can answer that question.
FTP links are not opening properly in Thunderbird 52.0 (32bit)

example: ftp://speedtest.tele2.net/5MB.zip

link is opened in Firefox as file:///C:/Users/-username-/AppData/Local/Microsoft/Windows/Temporary Internet Files/.....

can you please resolve this ?

Firefox 52.0.2 (64-bit) running on Windows 7 PRO
removing places.sqlite worked for me thanks everyone :)
Mac OS X 10.9.5 TB 52.0 with Lightning 5.4

Same problem, same error messages as OP.

After deleting the places.sqlite files it all worked

Before deleting I noticed:
places.sqlite, places.sqlite-wal, and places.sqlite.corrupt have the same "Date Modified" as the newly installed TB 52.0.
places.sqlite-wal and places.sqlite.corrupt have the same "Date Created" as the TB 52.0 "Date Modified" from the new installation

Active Plugins:
Account Colors	9.2	true	accountcolors@DW-dev
AddressBookTab	1.5	true	AddressBookTab@dischert.luc
CompactHeader	2.1.0	true	{58D4392A-842E-11DE-B51A-C7B855D89593}
Contact Tabs	2.6	true	contacttabs@janek.org
FoxClocks	4.2.3	true	{d37dc5d0-431d-44e5-8c91-49419370caa1}
Identity Chooser	1.9.3	true	identitychooser@janek.org
Just Restart!	1.0.5	true	{5afc0857-ce93-471c-9a00-b6247890dc1d}
Lightning	5.4	true	{e2fda1a4-762b-4020-b5ad-a41df1933103}
Mail Merge	4.6.1	true	mailmerge@example.net
Manually sort folders	1.1.1	true	tbsortfolders@xulforum.org
messagenotesplus	1.5.0.0	true	messagenotesplus@mozilla.org
MoreFunctionsForAddressBook	0.7.5	true	{3e17310d-82e8-4a43-bd2f-7c3055bfe589}
Provider for Google Calendar	3.1	true	{a62ef8ec-5fdc-40c2-873c-223b8a6925cc}
Quicktext	0.9.11.7	true	{8845E3B3-E8FB-40E2-95E9-EC40294818C4}
Silvermel and Charamel XT	2.0.1	true	silvermelxt@pardal.de
Toolbar Buttons	1.1.1-signed.1-signed	true	{03B08592-E5B4-45ff-A0BE-C1D975458688}
(In reply to Jorg K (GMT+2) from comment #21)
> My assumption is this: For some users the upgrade happened where the
> secondary files existed. Maybe I'm wrong, only a storage person can answer
> that question.

This doesn't matter, if Sqlite cannot merge the journal on shutdown, it will do on connection opening. There's no risk of corruption.

One thing you should do for sure here, is to wrap history addition in a try/catch. While it's a valuable feature, being able to open links is more valuable, thus it should always work even if something earlier fails.

Regarding the corruption, I need at least of these broken databases to be able to do any kind of guessing on the cause. There isn't any known incompatibility in upgrading from 45 to 52, but it's possible Sqlite itself became stricter to small corruptions that already existed. Or it's possible one of the schema changes between the 2 versions hit one of those existing corruptions and halted on it.

Another strange thing is that if the .corrupt file exists, the db should have been replaced with a pristine one, and then things should just work. At a maximum it should need an application restart.

Btw, I need either some db that reproduces the problem, or a db that doesn't work, you can send that to my through mail for privacy reasons.
Flags: needinfo?(mak77)
Attached patch 1354007-places-try-catch.patch (v1). (obsolete) — — Splinter Review
Marco, thanks as always for your comments. I've added a try/catch for now. Next time it happens, I'll see whether I can get those places.sqlite* files from a user. So far we've been telling them to delete the files. So the old rule applies: "Backup before delete" ;-)
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8856373 - Flags: review?(acelists)
Don't we all have backups of our Mailboxes? ;)
I have sent Marco a copy of the Database Files before and after the Upgrade. Btw, before the upgrade there was only the places.sqlite file. Only after the upgrade, there are those 3 extra files.

Claus
Comment on attachment 8856373 [details] [diff] [review]
1354007-places-try-catch.patch (v1).

Review of attachment 8856373 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/contentAreaClick.js
@@ +184,5 @@
> +        visitDate: Date.now() * 1000,
> +        transitionType: Components.interfaces.nsINavHistoryService.TRANSITION_LINK
> +      }]
> +    });
> +  } catch (ex) {}

Do we want to log this to the error console (Components.utils.reportError(ex)) so that the user could fix accumulating his history if he wants?
Attachment #8856373 - Flags: review?(acelists) → review+
I think logging is a good idea ;-)
Attachment #8856373 - Attachment is obsolete: true
Attachment #8856668 - Flags: review?(acelists)
Attachment #8856668 - Flags: review?(acelists) → review+
https://hg.mozilla.org/comm-central/rev/cd308c03dde86f876a7ccf1c5c6da40ed3b9feff

I'll leave this open until we fix this in TB 52, otherwise the bug will be invisible.
Target Milestone: --- → Thunderbird 55.0
Comment on attachment 8856668 [details] [diff] [review]
1354007-places-try-catch.patch (v2).

[Approval Request Comment]
Regression caused by (bug #): No ides, all we know is that deleting places.sqlite* fixes the problem.
User impact if declined: Some users can't click links.
Testing completed (on c-c, etc.): Manual.
Risk to taking this patch (and alternatives if risky):
Low risk, only adds try/catch.
Attachment #8856668 - Flags: approval-comm-esr52?
Attachment #8856668 - Flags: approval-comm-beta+
Attachment #8856668 - Flags: approval-comm-aurora+
Thank you for the files, they were extremely useful.
So far I've reduced the problem to MigrateV35Up() and in particular to the CreateMobileRoot() function. That schema upgrade was for Sync, and in particular to recreate the mobile root.

In the Thunderbird case, the bookmarks service has never been initialized, and the roots don't exist, and that confuses the code, that thinks the database is corrupt.
This is something we must take into account in any future migration.
I'll file a bug for the problem and try to uplift it as far as possible.
Note that, since the database gets replaced, I'd expect a simple application restart to fix the history problem (losing previous history though), if that's not the case, we may have a second bug to chase. So please test that and let me know.
There is something I don't completely understand in the AFTER file I received, it should have a newer user_version if it was created with thunderbird 52, it looks like a normal database created by Thunderbird 45.
Did you by chance downgrade and then generate a new places.sqlite with the old version?
Flags: needinfo?(bugzilla2)
Depends on: 1355414
(In reply to Marco Bonardo [::mak] from comment #38)
> Note that, since the database gets replaced, I'd expect a simple application
> restart to fix the history problem (losing previous history though), if
> that's not the case, we may have a second bug to chase. So please test that
> and let me know.
I placed the places.sqlite.corrupt file I sent you into my profile as places.sqlite. After that, I can't click links, and even if I restart, I still can't click links.

Starting again in a debug version still gives:
[5696] WARNING: The SQL statement 'SELECT 1 FROM moz_places WHERE url_hash = 0'
could not be compiled due to an error: no such column: url_hash: file c:/mozilla
-source/comm-central/mozilla/storage/mozStorageConnection.cpp, line 1153
[5696] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file c:
/mozilla-source/comm-central/mozilla/storage/mozStorageConnection.cpp, line 1614

although the file got replaced when running with the non-debug TB 52.

So looks like the replacement is also not right.
(In reply to Jorg K (GMT+2) from comment #40)
> So looks like the replacement is also not right.
I binary compared the original corrupt file with the replacement created by TB 52 ESR and they are *identical*.
(In reply to Marco Bonardo [::mak] from comment #39)
> There is something I don't completely understand in the AFTER file I
> received, it should have a newer user_version if it was created with
> thunderbird 52, it looks like a normal database created by Thunderbird 45.
> Did you by chance downgrade and then generate a new places.sqlite with the
> old version?

Hi Marco.
I never went back to v45. I still am on v52 and I just made a new profile in TB with a backup of my profile from before the upgrade. Then I sent you the original backupfile and the ones after the new profile was loaded into tb52.
But I guess Jorg's comments already solved the mystery ;)
Flags: needinfo?(bugzilla2)
(In reply to Jorg K (GMT+2) from comment #41)
> I binary compared the original corrupt file with the replacement created by
> TB 52 ESR and they are *identical*.

I suspect that when we call Remove on the database file, it fails because it's kept on hold by the OS, thus we don't really remove it. I don't have a solution for this, but I could probably set "places.database.replaceOnStartup" if the removal fails, and that  should take care of the issue at the next startup.
So let me summarise. Certain TB 45 users have a places.sqlite (comment #37: bookmarks service has never been initialized) that when upgrading to TB 52 will automatically get corrupted. That's not a nice position to be in.

Next the process that moves corrupt databases out of the way doesn't work, so the only workaround is to have a try/catch to kick on, even if the history can't be recorded. And the corrupted file will live forever with no history recorded any more. Equally not a nice position to be in.

I don't really understand your comment #43: The application is the only program which would have a lock on the file, so before removing it, it would first have to close it. What do you mean by "kept on hold by the OS"?
(In reply to Jorg K (GMT+2) from comment #44)
> So let me summarise. Certain TB 45 users have a places.sqlite (comment #37:
> bookmarks service has never been initialized) that when upgrading to TB 52
> will automatically get corrupted. That's not a nice position to be in.

Right.

> Next the process that moves corrupt databases out of the way doesn't work,
> so the only workaround is to have a try/catch to kick on, even if the
> history can't be recorded. And the corrupted file will live forever with no
> history recorded any more. Equally not a nice position to be in.

Not totally correct, once bug 1355414 will be fixed, the next startup should be able to migrate the database correctly. So we can exit from this situation. And will also add additional protection so that in the future if we are unable to remove the db on shutdown, we will do on startup.
You should try/catch regardless, because until the next app startup it's unlikely we can do anything, unless we find some magic solution.

> I don't really understand your comment #43: The application is the only
> program which would have a lock on the file, so before removing it, it would
> first have to close it. What do you mean by "kept on hold by the OS"?

There are 2 issues here:
1. Windows likes to keep handles to files open longer than needed, compared to other OS. For example if you look at output for xpcshell-tests, you can often see it's unable to remove some files from the scratch profile folder. We can't do much about this, the file looks locked, even if we closed our handle.
2. The whole Places (but also for most of the other consumers) migration path is synchronous, something WANTS to use the API now, so we must return a working handle immediately. Thus we can't wait for any asynchronous closing task. What we do currently is closing the database connection and then bailing out, but that's not always enough to unlock the file handle.

The only plausible solution that comes to my mind here would be to spin the events loop for 5 seconds until the removal succeeds, but that's another hack that may also cause unexpected crashes.
I will do some testing, but no promises that I can find a simple solution to fix the profile in the same session where it was marked is corrupt. I can surely make it fixable at the next run.
Comment on attachment 8856668 [details] [diff] [review]
1354007-places-try-catch.patch (v2).

https://hg.mozilla.org/releases/comm-esr52/rev/0b33f59988cf969b5f572e7d015355cac68e393c
Attachment #8856668 - Flags: approval-comm-esr52? → approval-comm-esr52+
Just a brief update.
The patch in bug 1355414 should allow profiles to properly migrate, even if they are stuck with the "corrupt" file. We just need to land it in central and then uplift to all thunderbird repos (I assume it's just comm-esr52?).
In future versions that patch will cause a forced crash and database replacement on the next start, rather than leaving the user stuck with a corrupt db.

I'm also working on bug 1355561 that should allow us to properly remove the corrupt database in most cases, that should reduce the likelihood of having to forcibly crash the user.
Marco, thanks for the update. We have a release branch THUNDERBIRD_52_VERBRANCH on M-esr52 to which I can uplift any M-* changes for release in TB 52 ESR.
Attachment #8856668 - Flags: approval-comm-beta+
Component: Untriaged → Message Reader UI
Let's mark this fixed now since the fix here shipped in TB 52.0.1. We'll still uplift bug 1355414.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Although this landed on beta channel, no build until 54 so it's not really fixed in 53
I can't micro-manage that. If it lands on a repo, it's fixed for that version by definition. If nothing ever ships, that's unfortunate. Otherwise I'd have to set those flags after successful release, not when landing :-(
(In reply to Jorg K (GMT+2) from comment #53)
> I can't micro-manage that. If it lands on a repo, it's fixed for that
> version by definition. If nothing ever ships, that's unfortunate. Otherwise


I understand. But the bug flags IMO are not about the repo record (hg is the record), the flags are for the record/reality for our _releases_. I'm just seeking accuracy so bug queries will be correct. 

> I'd have to set those flags after successful release, not when landing :-(
The solution there, in the general case, is to not land prior to a merge. I guess in this case and the other 4 bugs, you landed it for SM (comment 30). But again the bug flag is for us in TB, not for SM. So you can land without setting the TB flag to fixed
You need to log in before you can comment on or make changes to this bug.