462379 - (NS_ERROR_FAILURE) [nsINavBookmarksService.insertBookmark] when trying to star a bookmark

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P1)

Description

[Marcia Knous [:marcia]]

Seen while running  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081030 Minefield/3.1b2pre. Does not seem to happen using Win XP equivalent build. I am using a new profile.

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsINavBookmarksService.insertBookmark]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///Users/marcia/Desktop/Latest%20Trunk/Minefield.app/Contents/MacOS/components/nsPlacesTransactionsService.js :: PCIT_doTransaction :: line 377"  data: no]

Comment 1

[Marcia Knous [:marcia]]

hrm, after I tried to go back and reproduce this I have not yet been able to. Will keep trying.

Comment 2

[Shawn Wilsher :sdwilsh]

I don't expect you to be able to reproduce this consistently.  After talking with asuth for a few hours this morning, we think that this is the problem:

When we run our sync of both historyvistits and places on the background, we use the multiple statement API.  However, due to bugs like bug 462173, we can end up starting a transaction but never finishing it.  We end up not finishing it if we have a select statement that has not yet been reset on the main thread.

Additionally, there is a bug with the transaction code in mozStorageConnection that sets mTransactionInProgress to false even if an error occurred, resulting in the next use of mozStorageTransaction to succeed in obtaining a transaction, when it doesn't actually have one.

When we go ahead and try to commit our transaction on the main thread, it ends up failing, making this operation fail as well.

The simple fix here is to not use the multiple statement api when we sync both tables, and to dispatch them both as one-offs.

Comment 3

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

Here is that quick fix.

Comment 4

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

silly shawn - tricks are for kids

Comment 5

[Peter Lairo]

I'm getting these errors:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsINavBookmarksService.insertBookmark]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///C:/Program%20Files/Mozilla%20Firefox%20-%20Trunk/components/nsPlacesTransactionsService.js :: PCIT_doTransaction :: line 377"  data: no]

and

Error: Async statement execution returned with '1', 'Transaction failed to commit'
Source File: file:///C:/Program%20Files/Mozilla%20Firefox%20-%20Trunk/components/nsPlacesDBFlush.js
Line: 172

I'm using:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081030 Minefield/3.1b2pre

Comment 6

[Shawn Wilsher :sdwilsh]

Attachment: v1.2

I am full of fail today

Comment 7

[Dietrich Ayala (:dietrich)]

Comment on attachment 345564 [details] [diff] [review]
v1.2

r=me, thanks

Comment 8

[Shawn Wilsher :sdwilsh]

(In reply to comment #5)
> I'm getting these errors:
What you are seeing is exactly what I described in comment 2

Comment 9

[Shawn Wilsher :sdwilsh]

http://hg.mozilla.org/mozilla-central/rev/812455b0e56a

Comment 11

[Shawn Wilsher :sdwilsh]

tests are failing - I know why.  It's actually correct, but we should fix things slightly.

Comment 12

[Shawn Wilsher :sdwilsh]

Attachment: v2.0

This will batch the two completions we receive into one, as the code used to work.

Comment 13

[Dietrich Ayala (:dietrich)]

Comment on attachment 345606 [details] [diff] [review]
v2.0

checked in http://hg.mozilla.org/mozilla-central/rev/ec2b86038a39

Comment 14

[Ed Lee :Mardak]

Pushed extra followup:
http://hg.mozilla.org/mozilla-central/rev/74750cf8a8a7

       6          this._count++;
       7 -        if (this._count == 1) {
       8 +        if (this._count == 2) {
       9            // we have gotten both notifications

Comment 15

[Shawn Wilsher :sdwilsh]

(In reply to comment #14)
Quoted for emphasis:
> I am full of fail today

Comment 16

[Dietrich Ayala (:dietrich)]

Tests still failing on the tinderbox. I backed everything out.

Comment 17

[Marco Bonardo [:mak]]

mh, why are you checking on a fixed number of statements, the function actually could have one or two statements based on the fact we are syncing only moz_places or both tables, and the check for a fixed count does not make sense, you must use statements.length.

Comment 18

[Marco Bonardo [:mak]]

i would tell initialize _count to statements length and decrease till 0, should be less error prone.

Comment 19

[Shawn Wilsher :sdwilsh]

yeah, the solution in comment 18 is the correct one.  I clearly was not thinking correctly yesterday.

Comment 20

[Dietrich Ayala (:dietrich)]

Attachment: v2.1

Comment 21

[Shawn Wilsher :sdwilsh]

Comment on attachment 345747 [details] [diff] [review]
v2.1

r=sdwilsh

Comment 22

[Dietrich Ayala (:dietrich)]

checked in: http://hg.mozilla.org/mozilla-central/rev/944d15e482ba

will mark fixed after confirming the tests still pass.

Comment 23

[Shawn Wilsher :sdwilsh]

Tests have been passing, so I'm going to resolve this.

Comment 24

[Dietrich Ayala (:dietrich)]

is this error related? i'm still seeing this when i add a bookmark: 

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface]"  nsresult: "0x80004002 (NS_NOINTERFACE)"  location: "native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0"  data: no]

Comment 25

[Shawn Wilsher :sdwilsh]

It shouldn't be - Marco added code to make sure our QI is correct now...

Comment 26

[Tracy Walker [:tracy]]

verified: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.1b2pre) Gecko/20081112 Minefield/3.1b2pre

was there a new bug filed on comment #24 error.  I see it in the above build.

Comment 27

[Dietrich Ayala (:dietrich)]

(In reply to comment #26)
> verified: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.1b2pre)
> Gecko/20081112 Minefield/3.1b2pre
> 
> was there a new bug filed on comment #24 error.  I see it in the above build.

filed bug 464471

Comment 28

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv