596710 - Crash [@ nsBlockInFlowLineIterator::Prev ] [@ nsBlockInFlowLineIterator::Prev() ] when editing text fields (e.g. Bugzilla comments)

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Daniel Holbert [:dholbert]]

Filing a bug on a crash when editing large Bugzilla text fields (or other textfields, based on the "Comments" on matching crash reports)

I've crashed with this twice in the last hour -- once when typing review comments in the "edit attachment" page, and once when typing up a bug description in the "enter bug" page. I lost the earlier crash report, but the latter is:bp-0e5ecf83-d6bc-4426-9491-f48292100915

Setting this to block bug 240933, per ehsan's request.

Comment 1

[(no longer active)]

roc, is calling EnsureTextRun here <http://hg.mozilla.org/mozilla-central/annotate/0caec4ddff74/layout/generic/nsTextFrameThebes.cpp#l7138> always safe?

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to comment #0)
> I lost the earlier crash report

Ah, found it -- the earlier crash report is: bp-f569cabb-61d8-4147-baaf-fc37a2100915

(Stack looks identical to the one from comment 0)

Comment 3

[(no longer active)]

Daniel, do you have any idea what you were doing when you saw these crashes?  I'm looking for things like "just typing text", "moving the caret using arrow keys and then typing text", "moving the caret using the mouse and then typing text", ...  You get the idea.

Comment 4

[Daniel Holbert [:dholbert]]

The first time, I was editing a long bugzilla comment (which I'd mostly written in an external editor, and then copy/pasted in).  I believe the cursor was at the very end. I pressed page up a few times, and it paged up and then crashed.

The second time, I was typing up a bug description, and I somehow ended up in a state where the cursor disappeared (at the very bottom of the textfield).  I think I typed a little more, or maybe pressed some arrow keys, to try to resurrect the cursor, and then Firefox crashed.

Comment 5

[Daniel Holbert [:dholbert]]

er s/cursor/caret/

Comment 6

[(no longer active)]

(In reply to comment #4)
> The first time, I was editing a long bugzilla comment (which I'd mostly written
> in an external editor, and then copy/pasted in).  I believe the cursor was at
> the very end. I pressed page up a few times, and it paged up and then crashed.
> 
> The second time, I was typing up a bug description, and I somehow ended up in a
> state where the cursor disappeared (at the very bottom of the textfield).  I
> think I typed a little more, or maybe pressed some arrow keys, to try to
> resurrect the cursor, and then Firefox crashed.

So, in none of the cases were you actually typing something when the crash happened?  That's really strange, since the stack shows that the crash happened while typing...

Comment 7

[Daniel Holbert [:dholbert]]

The first crash, I was *intending* to type something at the top of the message, but I'm pretty sure I hadn't gotten that far -- I was partway through repeatedly pressing "page up", to get to the top. (I might be misremembering, though)

The second crash, I think I was indeed typing when it crashed.

Comment 8

[matthias koplenig [:metasieben]]

crashed for the 3 time in two days just now, 'cause of this. on both linux & mac.

don't know if it is related, but the cursor seems to act really strange while editing. jumping lines aka. adding invisible \n at the end of lines.

pasting text is also wonky, sometimes the pasted text doesn't show up, sometimes all newlines are stripped.

Comment 9

[Benjamin Smedberg]

dholbert, does this happen often enough that you might be able to catch it in a debugger or recording? This might be related to bug 490480, although that might be entirely unrelated which just happens to have the same signature.

Comment 10

[John Ford [:jhford] CET/CEST Berlin Time]

this has now hit me a few times today.  Every time it happened, I was editing a page on wiki.mozilla.org (PostImage).  The trigger seemed to be double clicking in the text box.  It seems to happen only after putting quite a bit of text in the text field.  Two times, i had dragged the size of the text field to be larger than default.

Comment 11

[Daniel Holbert [:dholbert]]

I just hit this for the third time today, with a slightly different crash stack:
bp-26a8d8c5-e43a-4708-83ab-e42482100915

I was running an opt build at the time, but I can try to catch this in a debugger.

FWIW, before each of my crashes, I think I've had odd behavior like what matthias mentioned in comment 8:
> the cursor seems to act really strange while
> editing. jumping lines aka. adding invisible \n at the end of lines.
For me, it often ends up being that I hit spacebar, and then my caret ends up on the next line (inserting a newline).  Then I'll click on the previous line, to move the caret back there -- and at some point later on, I'll hit backspace to delete an end-of-line space character, and the next newline will be deleted.  (So it's like a space character generates a "bonded" newline that dies when the space dies.)

The unexpected-newline seems to always be generated during or immediately after a GC pause (or some other sort of brief period of decreased responsiveness).

Comment 12

[Daniel Holbert [:dholbert]]

(In reply to comment #11)
> For me, it often ends up being that I hit spacebar, and then my caret ends up
> on the next line (inserting a newline).

Ah -- I figured out what's going on there. If the current line ends with a newline (i.e. if you're not on the final line of text), and you press 'end' and then spacebar, then the space character ends up at the beginning of the next line.  That's broken.  (I'm not sure that that's related to this crash, though -- probably needs a separate bug.)

(I also just spent 10-15 minutes pasting giant chunks of text & typing & seeking within a bugzilla textfield in a debug build, in an attempt to trigger this crash, but I wasn't able to.)

Comment 13

[Daniel C]

I hit this twice when editing a previously saved draft in Gmail. I can't work out a reliable set of steps, but something strange happens if you do the following:

1. Compose a new mail. Fill in To: and Subject:.
2. Type a few characters but only on one line. Don't create a new line.
3. Press Save Now.
4. Check that there's still only 1 line there.
5. Go to the Inbox.
6. Go back to Drafts and select the draft message.
7. The textarea is unfocused. Click somewhere towards the lower right of the textarea (you would expect the caret to end up after the last character.
8. See that the caret ends up at the beginning of the text, and a newline has been added at the end.

I can't get it to crash again yet, but both times that it did crash was after doing something similar to the above steps.

My stacks are different to dholberts but with the same signature. See bp-a6ba9bea-41ff-443e-a4d0-00c382100915.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Based on frequency I think we should bump this up.

(In reply to comment #1)
> roc, is calling EnsureTextRun here
> <http://hg.mozilla.org/mozilla-central/annotate/0caec4ddff74/layout/generic/nsTextFrameThebes.cpp#l7138>
> always safe?

I think so. It's not during reflow, so the frame tree should be intact.

Based on the stack, I think this is a regression from bug 593211. We're crashing in one of the begin() or begin_lines() calls in nsBlockInFlowLineIterator::Prev.

Comment 15

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

I also saw this, I crashed 4-6 times in the last 2 days while editing attachments as comments in bugzilla (for reviews). When it happened I was moving the cursor in the text field with directional keys and starting to type.
some link to my reports
http://crash-stats.mozilla.com/report/index/bp-14a35e3f-8ef1-478e-a717-e55672100915
http://crash-stats.mozilla.com/report/index/bp-5e80fddd-75b4-47cb-b4db-396382100914
http://crash-stats.mozilla.com/report/index/bp-db79558f-82d0-4e88-8bb2-5e2622100914

Comment 16

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

(In reply to comment #12)
> (In reply to comment #11)
> > For me, it often ends up being that I hit spacebar, and then my caret ends up
> > on the next line (inserting a newline).

confirmed here, worth filing a bug, it's pretty annoying

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

Is this affecting all systems?

Comment 18

[Stebs]

bp-0a0ddbce-6f08-409f-8e9b-1d50c2100916
bp-a28fca64-8d9a-4523-b6d3-eba612100916

I think I was typing fast both times, at least on one time, I hit Shift + w (for W)

Comment 19

[Daniel Holbert [:dholbert]]

(In reply to comment #16)
> > > For me, it often ends up being that I hit spacebar, and then my caret ends up
> > > on the next line (inserting a newline).
> 
> confirmed here, worth filing a bug, it's pretty annoying

I filed bug 593211 on that.

Comment 20

[(no longer active)]

(In reply to comment #16)> (In reply to comment #12)> > (In reply to comment #11)> > > For me, it often ends up being that I hit spacebar, and then my caret ends up> > > on the next line (inserting a newline).> > confirmed here, worth filing a bug, it's pretty annoyingThis is bug 596506, which will be fixed today.  I seriously doubt that it can lead to any sort of crash, though.

Comment 21

[Daniel Holbert [:dholbert]]

(In reply to comment #17)
> Is this affecting all systems?

We've got crash reports for this on Linux (mine & comment 13 & comment 18) and Windows (mak's in Comment 15).

So while there haven't been any Mac crash reports yet, I'd say "probably yes", this seems to affect all platforms.

Comment 22

[Daniel Holbert [:dholbert]]

(oops, I put the wrong new bug # in comment 19 -- meant to say "bug 597016" -- but I've now duped that to the one ehsan mentioned in comment 20 anyway)

Comment 23

[matthias koplenig [:metasieben]]

just to make this _sure_ ;)

i can confirm this crash on mac too.

http://crash-stats.mozilla.com/report/index/bp-8e56d148-1930-48a7-baa6-fb7262100915

Comment 24

[(no longer active)]

According to <http://crash-stats.mozilla.com/products/Firefox/versions/4.0b7pre>, this is the top crasher in b7pre builds after a "no signature" crash...

Comment 25

[Martijn Wargers (dead)]

Attachment: testcase

I'm seeing this crash with this testcase.
To reproduce:
- Click on the right-hand side of the textarea, just left from the scrollbar.
- Press the left arrow key and keep it pressed.

You should crash within one second with this stacktrace.

I'm also crashing with this stack:
http://crash-stats.mozilla.com/report/index/2aa0ec36-6501-46a7-95c9-cfa272100916
Hopefully, that's the same crash as this one, basically, because I don't really have time to minimize that one.

Comment 26

[John Ford [:jhford] CET/CEST Berlin Time]

I tried Martijn's steps and wasn't able to crash.  I did crash when I left the text area.

http://crash-stats.mozilla.com/report/index/f3b684a6-128b-4572-9d0c-293f62100916

Comment 27

[Daniel Holbert [:dholbert]]

(FWIW, I wasn't able to crash on Martijn's testcase, even after clicking out of the text area)

Comment 28

[(no longer active)]

I wasn't able to reproduce the crash using the test case, but I managed to file another weird problem, bug 597331...

Comment 29

[(no longer active)]

(In reply to comment #25)
> I'm also crashing with this stack:
> http://crash-stats.mozilla.com/report/index/2aa0ec36-6501-46a7-95c9-cfa272100916
> Hopefully, that's the same crash as this one, basically, because I don't really
> have time to minimize that one.

Oh, actually, this looks different.  I filed bug 597333 for it.

Comment 30

[Martijn Wargers (dead)]

Sorry, better str:
- Click on the right-hand side of the textarea, just left from the scrollbar.
- Press the left arrow key once, wait at least 10ms.
- Prss the left arrow key now at least 10 times.

Comment 31

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I can reproduce!

Comment 32

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Comment 33

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Martijn, it would be great if you can test this patch to make sure it fixes the bug.

Comment 34

[(no longer active)]

I'm not sure why, but I couldn't reproduce this even with comment 30.  But anyway, looking at roc's fix, |--rline| was clearly a mistake of mine.  I guess this would cause us to fail to find a valid line when we expected one to exist <http://hg.mozilla.org/mozilla-central/annotate/f38ef1080bfe/layout/generic/nsTextFrameThebes.cpp#l1068> and would cause us to crash when trying to access the invalid mLink member, right?

Comment 35

[(no longer active)]

(In reply to comment #26)
> I tried Martijn's steps and wasn't able to crash.  I did crash when I left the
> text area.
> 
> http://crash-stats.mozilla.com/report/index/f3b684a6-128b-4572-9d0c-293f62100916

Martijn, John, it would also be awesome if you guys can come up with a way to reproduce this crash (bug 597333).

Comment 36

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to comment #35)
> (In reply to comment #26)
> > I tried Martijn's steps and wasn't able to crash.  I did crash when I left the
> > text area.
> > 
> > http://crash-stats.mozilla.com/report/index/f3b684a6-128b-4572-9d0c-293f62100916
> 
> Martijn, John, it would also be awesome if you guys can come up with a way to
> reproduce this crash (bug 597333).

So far my steps to reproduce are to write something long and important into a text field.  I will see if I can figure out a real trigger.

Comment 37

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 476193 [details] [diff] [review]
fix

The patch looks fine (clearly fixes a searching bug), though it's not clear to me why this was causing a crash.  Was it because:

 (a) we were starting off on the first line, and therefore the --rline caused us not to search any lines other than the first because it made rline == rline_end right from the start, and

 (b) the calling code failed to check aFoundValidLine, and that somehow (still not sure how) led to a crash?

Do the constructors of nsBlockInFlowLineIterator obey the same invariant as Next and Prev.  For Next and Prev, the header says that after a false return, methods on the object should not be called again.  Should the same be true for aFoundValidLine in the constructor?  Maybe that should be documented if it's the case?  And should the caller in this case at least have an assertion that aFoundValidLine was true (if it's not checking it).

Comment 38

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #37)
> The patch looks fine (clearly fixes a searching bug), though it's not clear to
> me why this was causing a crash.  Was it because:
> 
>  (a) we were starting off on the first line, and therefore the --rline caused
> us not to search any lines other than the first because it made rline ==
> rline_end right from the start, and
> 
>  (b) the calling code failed to check aFoundValidLine, and that somehow (still
> not sure how) led to a crash?

Actually we were starting on the last line. --rline moves the iterator *forwards*, causing it to equal rline_end, so we didn't search any of the lines before the last line. That means we fail to find a valid line, triggering assertions in BuidTextRuns here:
  } else {
    backIterator = nsBlockInFlowLineIterator(block, aForFrame, &isValid);
    NS_ASSERTION(isValid, "aForFrame not found in block, someone lied to us");
    NS_ASSERTION(backIterator.GetContainer() == block,
                 "Someone lied to us about the block");
and then we crash because we don't have a valid line.

> Do the constructors of nsBlockInFlowLineIterator obey the same invariant as
> Next and Prev.  For Next and Prev, the header says that after a false return,
> methods on the object should not be called again.  Should the same be true for
> aFoundValidLine in the constructor?  Maybe that should be documented if it's
> the case?

OK, I'll add that comment.

> And should the caller in this case at least have an assertion that
> aFoundValidLine was true (if it's not checking it).

It does.

Comment 39

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Ehsan, I can reproduce this way 90% of times, maybe you can try with these steps:
1. enter a bugzilla comment textarea
2. press and hold Enter for about 10 seconds
3. press and hold shift+pagUp

Comment 40

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix v2

Comment 41

[Daniel Holbert [:dholbert]]

http://hg.mozilla.org/mozilla-central/rev/9788539de0f1

Comment 43

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows NT 6.1; rv:2.0b7pre) Gecko/20100919 Firefox/4.0b7pre