search

agl / xmpp-client

An XMPP client with OTR support
BSD 3-Clause "New" or "Revised" License
365 stars 71 forks source link

panic: runtime error: slice bounds out of range #57

Closed leif closed 10 years ago

leif commented 10 years ago

I just had my first-ever xmpp-client crash. Prior to crashing I'd had one OTR conversation, which the other party ended, and then a number of presence messages. Server is jabber.ccc.de.

panic: runtime error: slice bounds out of range

goroutine 22 [running]:
runtime.panic(0x826a860, 0x83e4aef)
    /usr/lib/go/src/pkg/runtime/panic.c:279 +0xe9
code.google.com/p/go.crypto/ssh/terminal.(*Terminal).writeLine(0x18610340, 0x1864b130, 0x1, 0x2a)
    /home/amnesia/Persistent/go/src/code.google.com/p/go.crypto/ssh/terminal/terminal.go:565 +0x24c
code.google.com/p/go.crypto/ssh/terminal.(*Terminal).handleKey(0x18610340, 0x20, 0x0, 0x0, 0x18610300)
    /home/amnesia/Persistent/go/src/code.google.com/p/go.crypto/ssh/terminal/terminal.go:550 +0x7ea
code.google.com/p/go.crypto/ssh/terminal.(*Terminal).readLine(0x18610340, 0x0, 0x0, 0x0, 0x0)
    /home/amnesia/Persistent/go/src/code.google.com/p/go.crypto/ssh/terminal/terminal.go:665 +0x4a4
code.google.com/p/go.crypto/ssh/terminal.(*Terminal).ReadLine(0x18610340, 0x0, 0x0, 0x0, 0x0)
    /home/amnesia/Persistent/go/src/code.google.com/p/go.crypto/ssh/terminal/terminal.go:639 +0xab
main.(*Input).ProcessCommands(0x18646d9c, 0x187bbe30)
    /home/amnesia/Persistent/go/src/github.com/agl/xmpp-client/input.go:358 +0x19a
created by main.main
    /home/amnesia/Persistent/go/src/github.com/agl/xmpp-client/ui.go:371 +0x1680

goroutine 16 [select]:
main.main()
    /home/amnesia/Persistent/go/src/github.com/agl/xmpp-client/ui.go:385 +0x52a3

goroutine 19 [finalizer wait, 18 minutes]:
runtime.park(0x806ebc0, 0x83e7444, 0x83e67e9)
    /usr/lib/go/src/pkg/runtime/proc.c:1369 +0x94
runtime.parkunlock(0x83e7444, 0x83e67e9)
    /usr/lib/go/src/pkg/runtime/proc.c:1385 +0x3f
runfinq()
    /usr/lib/go/src/pkg/runtime/mgc0.c:2644 +0xc5
runtime.goexit()
    /usr/lib/go/src/pkg/runtime/proc.c:1445

goroutine 20 [syscall]:
os/signal.loop()
    /usr/lib/go/src/pkg/os/signal/signal_unix.go:21 +0x24
created by os/signal.init·1
    /usr/lib/go/src/pkg/os/signal/signal_unix.go:27 +0x37

goroutine 21 [chan receive]:
main.func·002()
    /home/amnesia/Persistent/go/src/github.com/agl/xmpp-client/ui.go:200 +0x5b
created by main.main
    /home/amnesia/Persistent/go/src/github.com/agl/xmpp-client/ui.go:203 +0x2a6

goroutine 23 [IO wait, 6 minutes]:
net.runtime_pollWait(0xb7549478, 0x72, 0x0)
    /usr/lib/go/src/pkg/runtime/netpoll.goc:146 +0x62
net.(*pollDesc).Wait(0x1863e5b8, 0x72, 0x0, 0x0)
    /usr/lib/go/src/pkg/net/fd_poll_runtime.go:84 +0x48
net.(*pollDesc).WaitRead(0x1863e5b8, 0x0, 0x0)
    /usr/lib/go/src/pkg/net/fd_poll_runtime.go:89 +0x46
net.(*netFD).Read(0x1863e580, 0x18854000, 0x8000, 0x8000, 0x0, 0xb7548218, 0xb)
    /usr/lib/go/src/pkg/net/fd_unix.go:242 +0x28d
net.(*conn).Read(0x1860e2c0, 0x18854000, 0x8000, 0x8000, 0x0, 0x0, 0x0)
    /usr/lib/go/src/pkg/net/net.go:122 +0xc6
crypto/tls.(*block).readFromUntil(0x1869a0c0, 0xb75494e8, 0x1860e2c0, 0x5, 0x0, 0x0)
    /usr/lib/go/src/pkg/crypto/tls/conn.go:451 +0xb0
crypto/tls.(*Conn).readRecord(0x18694180, 0x17, 0x0, 0x0)
    /usr/lib/go/src/pkg/crypto/tls/conn.go:536 +0x1be
crypto/tls.(*Conn).Read(0x18694180, 0x18617000, 0x1000, 0x1000, 0x0, 0x0, 0x0)
    /usr/lib/go/src/pkg/crypto/tls/conn.go:901 +0x140
bufio.(*Reader).fill(0x187bb770)
    /usr/lib/go/src/pkg/bufio/bufio.go:97 +0x14c
bufio.(*Reader).ReadByte(0x187bb770, 0xb747008c, 0x0, 0x0)
    /usr/lib/go/src/pkg/bufio/bufio.go:199 +0x75
encoding/xml.(*Decoder).getc(0x18676c00, 0xb747dc8c)
    /usr/lib/go/src/pkg/encoding/xml/xml.go:851 +0x8d
encoding/xml.(*Decoder).rawToken(0x18676c00, 0x0, 0x0, 0x0, 0x0)
    /usr/lib/go/src/pkg/encoding/xml/xml.go:511 +0x12d
encoding/xml.(*Decoder).Token(0x18676c00, 0x0, 0x0, 0x0, 0x0)
    /usr/lib/go/src/pkg/encoding/xml/xml.go:246 +0x688
github.com/agl/xmpp.nextStart(0x18676c00, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
    /home/amnesia/Persistent/go/src/github.com/agl/xmpp/xmpp.go:627 +0x6a
github.com/agl/xmpp.next(0x1861bb90, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
    /home/amnesia/Persistent/go/src/github.com/agl/xmpp/xmpp.go:800 +0x89
github.com/agl/xmpp.(*Conn).Next(0x1861bb90, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
    /home/amnesia/Persistent/go/src/github.com/agl/xmpp/xmpp.go:102 +0x6a
main.(*Session).readMessages(0x18646d80, 0x187bbe60)
    /home/amnesia/Persistent/go/src/github.com/agl/xmpp-client/ui.go:170 +0x4e
created by main.main
    /home/amnesia/Persistent/go/src/github.com/agl/xmpp-client/ui.go:374 +0x16ca
leif commented 10 years ago

This was with the latest master, 69c407829265982f968a3e84f3e38d1e7dc3d2aa

ioerror commented 10 years ago

Do you have a log file? Can you enable logging and see if it happens again?

leif commented 10 years ago

I don't have a log file. I'll enable logging.

I remembered that I did something else in this session before the OTR conversation: I attempted to start an OTR session with someone I don't know who requested presence notifications from me some time ago and I received the "?OTRv2?" error from the server (as has happened previous times I've attempted to start an OTR session with them).

agl commented 10 years ago

Here's the crashing code in question:

func (t *Terminal) writeLine(line []rune) {
    for len(line) != 0 {
        remainingOnLine := t.termWidth - t.cursorX
        todo := len(line)
        if todo > remainingOnLine {
            todo = remainingOnLine
        }
        t.queue(line[:todo])
        t.advanceCursor(visualLength(line[:todo]))
        line = line[todo:]
    }
}

And it's crashing on t.queue(line[:todo]). So either cursorX > termWidth or there's a race. I can't see that the former can happen so can you build xmpp-client with go build -race and use that binary? It should dump any detected races. Normally the race detector is quite a lot of overhead, but for a network-bound program with xmpp-client should should be almost invisible.

isislovecruft commented 10 years ago

@leif, by any chance, were you using a tiling window manager on that system that had the crash? Can you open a new terminal with bash in it and do trap -- 'echo size changed' SIGWINCH, and then try resizing the window in various ways (with hotkeys and/or mouse or whatever your WM uses) and see what will cause the "size changed" message to print?

Update: That trap has trouble triggering even on a super normal Ubuntu 14.04 system with Gnome and gnome-terminal, so it seems to not be limited to tiling WMs.

@agl I think there is a way in which the updateTerminalWidth handler for SIGWINCH is not properly getting called (because the signal isn't happening), causing, in some cases, cursorX > termWidth.

leif commented 10 years ago

I had this crash a second time a few days ago. I unfortunately haven't built with -race yet, but I did enable logs. Strangely though, the log seemed to have stopped being written some time before the crash (based on the pattern of sent and received messages, it seemed to be missing my last 10+ minutes of conversation).

@isislovecruft yes, I'm using dwm. It appears that SIGWINCH is firing once each time the window size is changed by any means (including when it jumps from a large size to a very small size).

I also tried jumping the window to a very small size while the cursor is very far to the right; this did not reproduce the crash but does cause several lines of whitespace to be incorrectly inserted above the current input line.

schulze commented 10 years ago

I also tried jumping the window to a very small size while the cursor is very far to the right; this did not reproduce the crash but does cause several lines of whitespace to be incorrectly inserted above the current input line.

Does using or not using UTF-8/Unicode characters make a difference?

leif commented 10 years ago

I just tried a few experiments resizing with a long line of unicode in the input buffer and haven't reproduced the crash yet. But, it is sometimes overwriting previous lines of conversation like it used to do before the semi-recent terminal library fixes.

leif commented 10 years ago

I can reliably reproduce this crash now with these two steps:

  1. talk to someone who's name is 19 characters long, so that the prompt is 21 bytes long (their name + > + space) so the cursor is in the 22nd column (counting from 1).
  2. move the window to a region where it becomes 18 columns wide (this is a single step, so there should be only one SIGWINCH)
leif commented 10 years ago

Another note: the traceback I posted originally was from an old build (69c407829265982f968a3e84f3e38d1e7dc3d2aa), but I just confirmed that the bug is still reproducible using the latest version (919dd8dcc9cd6d45200609064b5b3ca0e305f99c) which includes @isislovecruft's changes to the prompt.

leif commented 10 years ago

I found a much easier way to reproduce this crash, and without using dwm (I tested with metacity; I assume it is not window-manager dependent):

Simply resize the window so that it is narrower than the prompt, and then type something.

agl commented 10 years ago

https://codereview.appspot.com/158090043

agl commented 10 years ago

go get -u code.google.com/p/go.crypto/ssh/terminal and rebuilding Pond should fix this.

leif commented 10 years ago

rebuilding Pond should fix this

rebuilding xmpp-client helps too :)

Fix works for me. Thanks!