After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 787283 - Crash on cat hyperlink-demo.txt
Crash on cat hyperlink-demo.txt
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
git master
Other Linux
: Normal critical
: vte-0-50
Assigned To: VTE Maintainers
VTE Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-09-05 00:06 UTC by Egmont Koblinger
Modified: 2017-09-07 16:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix (973 bytes, patch)
2017-09-05 13:07 UTC, Egmont Koblinger
committed Details | Review

Description Egmont Koblinger 2017-09-05 00:06:11 UTC
Open g-t. It happens to open in the lower left of my desktop whereas my mouse pointer is somewhere outside of this area.

Drag the upper-right corner and resize to 150x50.

cat vte/perf/hyperlink-demo.txt

crash :(

Reproducibility: pretty much always crashes for a couple of minutes, then doesn't crash for another few minutes etc...

Just a wild guess, it _might_ be related to the mouse never entering the vte widget's area.

No clue at this moment whether the problem is in vte or g-t. Couldn't yet reproduce with testvte.
Comment 1 Egmont Koblinger 2017-09-05 06:25:36 UTC
(gdb) bt
  • #0 __GI_raise
    at ../sysdeps/unix/sysv/linux/raise.c line 58
  • #1 __GI_abort
    at abort.c line 89
  • #2 g_assertion_message
  • #3 g_assertion_message_expr
  • #4 VteTerminalPrivate::cursor_inside_match(vte::view::coords const&)
    at vte.cc line 5700
  • #5 0x00000000000000c6 in
  • #6 0x00007fffd2ea1e30 in
  • #7 0x0000556f4a49d850 in
  • #8 0x0000556f4a49d850 in
  • #9 0x000000000000003c in
  • #10 0x0000000000000000 in

Comment 2 Egmont Koblinger 2017-09-05 06:43:40 UTC
Vte:ERROR:vte.cc:5576:void VteTerminalPrivate::hyperlink_invalidate_and_get_bbox(hyperlink_idx_t, GdkRectangle*): assertion failed: (top != LONG_MAX && bottom != -1 && left != LONG_MAX && right != -1)
Comment 3 Egmont Koblinger 2017-09-05 13:05:25 UTC
hyperlink_hilite_update() is getting called plenty of times via emit_pending_signals()'s m_contents_changed_pending branch, taking m_mouse_last_position as the coordinate.

This coordinate is (0, 0) lot of times in a row. Then once, I can't yet see why, it becomes (-1, -1).

hyperlink_hilite_update() converts this to logical row and column, which becomes the logical row just above the viewport (i.e. the first scrolled out line).

Then it calls m_hyperlink_hover_idx = _vte_ring_get_hyperlink_at_position(...) to get the index of the "active" hyperlink, the one that's just been scrolled out.

Probably there's some timing heuristics in when it happens exactly (when the update cycle kicks in during cating the file). Sometimes there's no hyperlink found there, the index becomes 0 and the rest is okay. Sometimes however there's a hyperlink there and hence the index becomes nonzero. In the latter case:

hyperlink_invalidate_and_get_bbox() is called to get the bounding box of this new active hyperlink. This method has an assertion that the given hyperlink is indeed found onscreen, this is what fails.

There's lot to improve here, see bug 732185. Ideally, the mouse position should be allowed to be (-1, -1) -- or at least there needs to be a way to denote off-canvas coordinates, and this is the obvious one. The mouse moving over the padding, or leaving the vte widget should set this special value (this to be addressed in that bug).
Comment 4 Egmont Koblinger 2017-09-05 13:07:50 UTC
Created attachment 359185 [details] [review]
Fix

As for now, for a last-minute fix for 0.50, I'd prefer to go defensive and allow and tolerate (-1, -1) at that position. I don't know how it gets there now, but in the future it'll intentionally get there, so I don't care too much :)

I've tested it manually about 20 times and no crash.
Comment 5 Egmont Koblinger 2017-09-05 13:14:36 UTC
Just for the record: my wild guess seems to have been correct. In order to reproduce the bug, it's important for the mouse not to enter the widget's area.
Comment 6 Christian Persch 2017-09-05 18:31:59 UTC
Comment on attachment 359185 [details] [review]
Fix

Thanks! Ok to commit once you get r-t approval for the freeze break.
Comment 7 Egmont Koblinger 2017-09-05 21:04:20 UTC
Comment on attachment 359185 [details] [review]
Fix

Got the approval. Submitted.
Comment 8 Egmont Koblinger 2017-09-07 16:22:11 UTC
Just for the record: The crash used to occur 100% reliably at window sizes where after cat'ing the entire file (rather than during an intermediate display update) a hyperlink was present in the left column of most recently scrolled out line. I should've realized this earlier for easier testing :-) Anyway, with this sudden realization, I can now even more confidently confirm that it's fixed.