diff options
42 files changed, 1236 insertions, 158 deletions
@@ -10,7 +10,9 @@ # Go package. # My path to the X protocol XML descriptions. +ifndef XPROTO XPROTO=/usr/share/xcb +endif # All of the XML files in my /usr/share/xcb directory EXCEPT XKB. -_- # This is intended to build xgbgen and generate Go code for each supported @@ -15,10 +15,18 @@ Please see doc.go for more info. Note that unless you know you need XGB, you can probably make your life easier by using a slightly higher level library: xgbutil. +This is a fork of github.com/BurntSushi/xgb + Quick Usage =========== -go get github.com/BurntSushi/xgb -go run go/path/src/github.com/BurntSushi/xgb/examples/create-window/main.go +go get github.com/jezek/xgb +go run go/path/src/github.com/jezek/xgb/examples/create-window/main.go + +jezek's Fork +============ +I've forked the XGB repository from BurntSushi's github to apply some +patches which caused panics and memory leaks upon close and tests were added, +to test multiple server close scenarios. BurntSushi's Fork ================= diff --git a/bigreq/bigreq.go b/bigreq/bigreq.go index 6590376..d8a5859 100644 --- a/bigreq/bigreq.go +++ b/bigreq/bigreq.go @@ -4,9 +4,9 @@ package bigreq // This file is automatically generated from bigreq.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the BIG-REQUESTS extension. diff --git a/composite/composite.go b/composite/composite.go index 1373f8b..4622cee 100644 --- a/composite/composite.go +++ b/composite/composite.go @@ -4,10 +4,10 @@ package composite // This file is automatically generated from composite.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xfixes" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xfixes" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the Composite extension. @@ -2,6 +2,7 @@ package xgb import ( "errors" + "io" ) // Cookie is the internal representation of a cookie, where one is generated @@ -80,6 +81,7 @@ func (c Cookie) Reply() ([]byte, error) { // channels. If the former arrives, the bytes are returned with a nil error. // If the latter arrives, no bytes are returned (nil) and the error received // is returned. +// Returns (nil, io.EOF) when the connection is closed. // // Unless you're building requests from bytes by hand, this method should // not be used. @@ -98,6 +100,9 @@ func (c Cookie) replyChecked() ([]byte, error) { return reply, nil case err := <-c.errorChan: return nil, err + case <-c.conn.doneRead: + // c.conn.readResponses is no more, there will be no replys or errors + return nil, io.EOF } } @@ -106,6 +111,7 @@ func (c Cookie) replyChecked() ([]byte, error) { // If the latter arrives, no bytes are returned (nil) and a nil error // is returned. (In the latter case, the corresponding error can be retrieved // from (Wait|Poll)ForEvent asynchronously.) +// Returns (nil, io.EOF) when the connection is closed. // In all honesty, you *probably* don't want to use this method. // // Unless you're building requests from bytes by hand, this method should @@ -121,6 +127,9 @@ func (c Cookie) replyUnchecked() ([]byte, error) { return reply, nil case <-c.pingChan: return nil, nil + case <-c.conn.doneRead: + // c.conn.readResponses is no more, there will be no replys or pings + return nil, io.EOF } } @@ -132,6 +141,7 @@ func (c Cookie) replyUnchecked() ([]byte, error) { // Thus, pingChan is sent a value when the *next* reply is read. // If no more replies are being processed, we force a round trip request with // GetInputFocus. +// Returns io.EOF error when the connection is closed. // // Unless you're building requests from bytes by hand, this method should // not be used. @@ -161,5 +171,8 @@ func (c Cookie) Check() error { return err case <-c.pingChan: return nil + case <-c.conn.doneRead: + // c.conn.readResponses is no more, there will be no errors or pings + return io.EOF } } diff --git a/damage/damage.go b/damage/damage.go index 26eca04..53326d9 100644 --- a/damage/damage.go +++ b/damage/damage.go @@ -4,10 +4,10 @@ package damage // This file is automatically generated from damage.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xfixes" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xfixes" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the DAMAGE extension. @@ -10,7 +10,7 @@ Most uses of XGB typically fall under the realm of window manager and GUI kit development, but other applications (like pagers, panels, tilers, etc.) may also require XGB. Moreover, it is a near certainty that if you need to work with X, xgbutil will be of great use to you as well: -https://github.com/BurntSushi/xgbutil +https://github.com/jezek/xgbutil Example @@ -23,8 +23,8 @@ accompanying documentation can be found in examples/create-window. import ( "fmt" - "github.com/BurntSushi/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb" + "github.com/jezek/xgb/xproto" ) func main() { @@ -74,8 +74,8 @@ can be found in examples/xinerama. import ( "fmt" "log" - "github.com/BurntSushi/xgb" - "github.com/BurntSushi/xgb/xinerama" + "github.com/jezek/xgb" + "github.com/jezek/xgb/xinerama" ) func main() { diff --git a/dpms/dpms.go b/dpms/dpms.go index 4bf5883..da28f1e 100644 --- a/dpms/dpms.go +++ b/dpms/dpms.go @@ -4,9 +4,9 @@ package dpms // This file is automatically generated from dpms.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the DPMS extension. diff --git a/dri2/dri2.go b/dri2/dri2.go index 820cf2b..1cc1d0a 100644 --- a/dri2/dri2.go +++ b/dri2/dri2.go @@ -4,9 +4,9 @@ package dri2 // This file is automatically generated from dri2.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the DRI2 extension. diff --git a/examples/atoms/main.go b/examples/atoms/main.go index 8985768..b94a772 100644 --- a/examples/atoms/main.go +++ b/examples/atoms/main.go @@ -9,8 +9,8 @@ import ( "runtime/pprof" "time" - "github.com/BurntSushi/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb" + "github.com/jezek/xgb/xproto" ) var ( diff --git a/examples/create-window/main.go b/examples/create-window/main.go index 73a0099..50a9bff 100644 --- a/examples/create-window/main.go +++ b/examples/create-window/main.go @@ -7,8 +7,8 @@ package main import ( "fmt" - "github.com/BurntSushi/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb" + "github.com/jezek/xgb/xproto" ) func main() { diff --git a/examples/doc.go b/examples/doc.go index 80ea5b7..b516851 100644 --- a/examples/doc.go +++ b/examples/doc.go @@ -10,8 +10,8 @@ to events. If you're looking to query information about your window manager, get-active-window is a start. However, to do anything extensive requires -a lot of boiler plate. To that end, I'd recommend use of my higher level -library, xgbutil: https://github.com/BurntSushi/xgbutil +a lot of boiler plate. To that end, I'd recommend use of a higher level +library (eg. xgbutil: https://github.com/jezek/xgbutil). There are also examples of using the Xinerama and RandR extensions, if you're interested in querying information about your active heads. In RandR's case, diff --git a/examples/get-active-window/main.go b/examples/get-active-window/main.go index 48e020c..37f374c 100644 --- a/examples/get-active-window/main.go +++ b/examples/get-active-window/main.go @@ -6,8 +6,8 @@ import ( "fmt" "log" - "github.com/BurntSushi/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb" + "github.com/jezek/xgb/xproto" ) func main() { diff --git a/examples/randr/main.go b/examples/randr/main.go index e349144..13b934a 100644 --- a/examples/randr/main.go +++ b/examples/randr/main.go @@ -14,9 +14,9 @@ import ( "fmt" "log" - "github.com/BurntSushi/xgb" - "github.com/BurntSushi/xgb/randr" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb" + "github.com/jezek/xgb/randr" + "github.com/jezek/xgb/xproto" ) func main() { diff --git a/examples/xinerama/main.go b/examples/xinerama/main.go index 896bb63..a7e1531 100644 --- a/examples/xinerama/main.go +++ b/examples/xinerama/main.go @@ -5,8 +5,8 @@ import ( "fmt" "log" - "github.com/BurntSushi/xgb" - "github.com/BurntSushi/xgb/xinerama" + "github.com/jezek/xgb" + "github.com/jezek/xgb/xinerama" ) func main() { @@ -4,9 +4,9 @@ package ge // This file is automatically generated from ge.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the Generic Event Extension extension. @@ -4,9 +4,9 @@ package glx // This file is automatically generated from glx.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the GLX extension. diff --git a/randr/randr.go b/randr/randr.go index 021c011..bc62d73 100644 --- a/randr/randr.go +++ b/randr/randr.go @@ -4,10 +4,10 @@ package randr // This file is automatically generated from randr.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/render" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/render" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the RANDR extension. diff --git a/record/record.go b/record/record.go index 5469170..419587f 100644 --- a/record/record.go +++ b/record/record.go @@ -4,9 +4,9 @@ package record // This file is automatically generated from record.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the RECORD extension. diff --git a/render/render.go b/render/render.go index e15bd67..223162a 100644 --- a/render/render.go +++ b/render/render.go @@ -4,9 +4,9 @@ package render // This file is automatically generated from render.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the RENDER extension. @@ -4,9 +4,9 @@ package res // This file is automatically generated from res.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the X-Resource extension. diff --git a/screensaver/screensaver.go b/screensaver/screensaver.go index 418576c..6854505 100644 --- a/screensaver/screensaver.go +++ b/screensaver/screensaver.go @@ -4,9 +4,9 @@ package screensaver // This file is automatically generated from screensaver.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the MIT-SCREEN-SAVER extension. diff --git a/shape/shape.go b/shape/shape.go index 7069f7e..42b3f9f 100644 --- a/shape/shape.go +++ b/shape/shape.go @@ -4,9 +4,9 @@ package shape // This file is automatically generated from shape.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the SHAPE extension. @@ -4,9 +4,9 @@ package shm // This file is automatically generated from shm.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the MIT-SHM extension. diff --git a/testingTools.go b/testingTools.go new file mode 100644 index 0000000..2f73031 --- /dev/null +++ b/testingTools.go @@ -0,0 +1,426 @@ +package xgb + +import ( + "bytes" + "errors" + "io" + "net" + "regexp" + "runtime" + "strconv" + "strings" + "testing" + "time" +) + +// Leaks monitor + +type goroutine struct { + id int + name string + stack []byte +} + +type leaks struct { + name string + goroutines map[int]goroutine + report []*leaks +} + +func leaksMonitor(name string, monitors ...*leaks) *leaks { + return &leaks{ + name, + leaks{}.collectGoroutines(), + monitors, + } +} + +// ispired by https://golang.org/src/runtime/debug/stack.go?s=587:606#L21 +// stack returns a formatted stack trace of all goroutines. +// It calls runtime.Stack with a large enough buffer to capture the entire trace. +func (_ leaks) stack() []byte { + buf := make([]byte, 1024) + for { + n := runtime.Stack(buf, true) + if n < len(buf) { + return buf[:n] + } + buf = make([]byte, 2*len(buf)) + } +} + +func (l leaks) collectGoroutines() map[int]goroutine { + res := make(map[int]goroutine) + stacks := bytes.Split(l.stack(), []byte{'\n', '\n'}) + + regexpId := regexp.MustCompile(`^\s*goroutine\s*(\d+)`) + for _, st := range stacks { + lines := bytes.Split(st, []byte{'\n'}) + if len(lines) < 2 { + panic("routine stach has less tnan two lines: " + string(st)) + } + + idMatches := regexpId.FindSubmatch(lines[0]) + if len(idMatches) < 2 { + panic("no id found in goroutine stack's first line: " + string(lines[0])) + } + id, err := strconv.Atoi(string(idMatches[1])) + if err != nil { + panic("converting goroutine id to number error: " + err.Error()) + } + if _, ok := res[id]; ok { + panic("2 goroutines with same id: " + strconv.Itoa(id)) + } + name := strings.TrimSpace(string(lines[1])) + + //filter out our stack routine + if strings.Contains(name, "xgb.leaks.stack") { + continue + } + + res[id] = goroutine{id, name, st} + } + return res +} + +func (l leaks) leakingGoroutines() []goroutine { + goroutines := l.collectGoroutines() + res := []goroutine{} + for id, gr := range goroutines { + if _, ok := l.goroutines[id]; ok { + continue + } + res = append(res, gr) + } + return res +} +func (l leaks) checkTesting(t *testing.T) { + if len(l.leakingGoroutines()) == 0 { + return + } + leakTimeout := 10 * time.Millisecond + time.Sleep(leakTimeout) + //t.Logf("possible goroutine leakage, waiting %v", leakTimeout) + grs := l.leakingGoroutines() + for _, gr := range grs { + t.Errorf("%s: %s is leaking", l.name, gr.name) + //t.Errorf("%s: %s is leaking\n%v", l.name, gr.name, string(gr.stack)) + } + for _, rl := range l.report { + rl.ignoreLeak(grs...) + } +} +func (l *leaks) ignoreLeak(grs ...goroutine) { + for _, gr := range grs { + l.goroutines[gr.id] = gr + } +} + +// dummy net.Conn + +type dAddr struct { + s string +} + +func (_ dAddr) Network() string { return "dummy" } +func (a dAddr) String() string { return a.s } + +var ( + dNCErrNotImplemented = errors.New("command not implemented") + dNCErrClosed = errors.New("server closed") + dNCErrWrite = errors.New("server write failed") + dNCErrRead = errors.New("server read failed") + dNCErrResponse = errors.New("server response error") +) + +type dNCIoResult struct { + n int + err error +} +type dNCIo struct { + b []byte + result chan dNCIoResult +} + +type dNCCWriteLock struct{} +type dNCCWriteUnlock struct{} +type dNCCWriteError struct{} +type dNCCWriteSuccess struct{} +type dNCCReadLock struct{} +type dNCCReadUnlock struct{} +type dNCCReadError struct{} +type dNCCReadSuccess struct{} + +// dummy net.Conn interface. Needs to be constructed via newDummyNetConn([...]) function. +type dNC struct { + reply func([]byte) []byte + addr dAddr + in, out chan dNCIo + control chan interface{} + done chan struct{} +} + +// Results running dummy server, satisfying net.Conn interface for test purposes. +// 'name' parameter will be returned via (*dNC).Local/RemoteAddr().String() +// 'reply' parameter function will be runned only on successful (*dNC).Write(b) with 'b' as parameter to 'reply'. The result will be stored in internal buffer and can be retrieved later via (*dNC).Read([...]) method. +// It is users responsibility to stop and clean up resources with (*dNC).Close, if not needed anymore. +// By default, the (*dNC).Write([...]) and (*dNC).Read([...]) methods are unlocked and will not result in error. +//TODO make (*dNC).SetDeadline, (*dNC).SetReadDeadline, (*dNC).SetWriteDeadline work proprely. +func newDummyNetConn(name string, reply func([]byte) []byte) *dNC { + + s := &dNC{ + reply, + dAddr{name}, + make(chan dNCIo), make(chan dNCIo), + make(chan interface{}), + make(chan struct{}), + } + + in, out := s.in, chan dNCIo(nil) + buf := &bytes.Buffer{} + errorRead, errorWrite := false, false + lockRead := false + + go func() { + defer close(s.done) + for { + select { + case dxsio := <-in: + if errorWrite { + dxsio.result <- dNCIoResult{0, dNCErrWrite} + break + } + + response := s.reply(dxsio.b) + + buf.Write(response) + dxsio.result <- dNCIoResult{len(dxsio.b), nil} + + if !lockRead && buf.Len() > 0 && out == nil { + out = s.out + } + case dxsio := <-out: + if errorRead { + dxsio.result <- dNCIoResult{0, dNCErrRead} + break + } + + n, err := buf.Read(dxsio.b) + dxsio.result <- dNCIoResult{n, err} + + if buf.Len() == 0 { + out = nil + } + case ci := <-s.control: + if ci == nil { + return + } + switch ci.(type) { + case dNCCWriteLock: + in = nil + case dNCCWriteUnlock: + in = s.in + case dNCCWriteError: + errorWrite = true + case dNCCWriteSuccess: + errorWrite = false + case dNCCReadLock: + out = nil + lockRead = true + case dNCCReadUnlock: + lockRead = false + if buf.Len() > 0 && out == nil { + out = s.out + } + case dNCCReadError: + errorRead = true + case dNCCReadSuccess: + errorRead = false + default: + } + } + } + }() + return s +} + +// Shuts down dummy net.Conn server. Every blocking or future method calls will do nothing and result in error. +// Result will be dNCErrClosed if server was allready closed. +// Server can not be unclosed. +func (s *dNC) Close() error { + select { + case s.control <- nil: + <-s.done + return nil + case <-s.done: + } + return dNCErrClosed +} + +// Performs a write action to server. +// If not locked by (*dNC).WriteLock, it results in error or success. If locked, this method will block until unlocked, or closed. +// +// This method can be set to result in error or success, via (*dNC).WriteError() or (*dNC).WriteSuccess() methods. +// +// If setted to result in error, the 'reply' function will NOT be called and internal buffer will NOT increasethe. +// Result will be (0, dNCErrWrite). +// +// If setted to result in success, the 'reply' function will be called and its result will be writen to internal buffer. +// If there is something in the internal buffer, the (*dNC).Read([...]) will be unblocked (if not previously locked with (*dNC).ReadLock). +// Result will be (len(b), nil) +// +// If server was closed previously, result will be (0, dNCErrClosed). +func (s *dNC) Write(b []byte) (int, error) { + resChan := make(chan dNCIoResult) + select { + case s.in <- dNCIo{b, resChan}: + res := <-resChan + return res.n, res.err + case <-s.done: + } + return 0, dNCErrClosed +} + +// Performs a read action from server. +// If locked by (*dNC).ReadLock(), this method will block until unlocked with (*dNC).ReadUnlock(), or server closes. +// +// If not locked, this method can be setted to result imidiatly in error, will block if internal buffer is empty or will perform an read operation from internal buffer. +// +// If setted to result in error via (*dNC).ReadError(), the result will be (0, dNCErrWrite). +// +// If not locked and not setted to result in error via (*dNC).ReadSuccess(), this method will block until internall buffer is not empty, than it returns the result of the buffer read operation via (*bytes.Buffer).Read([...]). +// If the internal buffer is empty after this method, all follwing (*dNC).Read([...]), requests will block until internall buffer is filled after successful write requests. +// +// If server was closed previously, result will be (0, io.EOF). +func (s *dNC) Read(b []byte) (int, error) { + resChan := make(chan dNCIoResult) + select { + case s.out <- dNCIo{b, resChan}: + res := <-resChan + return res.n, res.err + case <-s.done: + } + return 0, io.EOF +} +func (s *dNC) LocalAddr() net.Addr { return s.addr } +func (s *dNC) RemoteAddr() net.Addr { return s.addr } +func (s *dNC) SetDeadline(t time.Time) error { return dNCErrNotImplemented } +func (s *dNC) SetReadDeadline(t time.Time) error { return dNCErrNotImplemented } +func (s *dNC) SetWriteDeadline(t time.Time) error { return dNCErrNotImplemented } + +func (s *dNC) Control(i interface{}) error { + select { + case s.control <- i: + return nil + case <-s.done: + } + return dNCErrClosed +} + +// Locks writing. All write requests will be blocked until write is unlocked with (*dNC).WriteUnlock, or server closes. +func (s *dNC) WriteLock() error { + return s.Control(dNCCWriteLock{}) +} + +// Unlocks writing. All blocked write requests until now will be accepted. +func (s *dNC) WriteUnlock() error { + return s.Control(dNCCWriteUnlock{}) +} + +// Unlocks writing and makes (*dNC).Write to result (0, dNCErrWrite). +func (s *dNC) WriteError() error { + if err := s.WriteUnlock(); err != nil { + return err + } + return s.Control(dNCCWriteError{}) +} + +// Unlocks writing and makes (*dNC).Write([...]) not result in error. See (*dNC).Write for details. +func (s *dNC) WriteSuccess() error { + if err := s.WriteUnlock(); err != nil { + return err + } + return s.Control(dNCCWriteSuccess{}) +} + +// Locks reading. All read requests will be blocked until read is unlocked with (*dNC).ReadUnlock, or server closes. +// (*dNC).Read([...]) wil block even after successful write. +func (s *dNC) ReadLock() error { + return s.Control(dNCCReadLock{}) +} + +// Unlocks reading. If the internall buffer is not empty, next read will not block. +func (s *dNC) ReadUnlock() error { + return s.Control(dNCCReadUnlock{}) +} + +// Unlocks read and makes every blocked and following (*dNC).Read([...]) imidiatly result in error. See (*dNC).Read for details. +func (s *dNC) ReadError() error { + if err := s.ReadUnlock(); err != nil { + return err + } + return s.Control(dNCCReadError{}) +} + +// Unlocks read and makes every blocked and following (*dNC).Read([...]) requests be handled, if according to internal buffer. See (*dNC).Read for details. +func (s *dNC) ReadSuccess() error { + if err := s.ReadUnlock(); err != nil { + return err + } + return s.Control(dNCCReadSuccess{}) +} + +// dummy X server replier for dummy net.Conn + +type dXSEvent struct{} + +func (_ dXSEvent) Bytes() []byte { return nil } +func (_ dXSEvent) String() string { return "dummy X server event" } + +type dXSError struct { + seqId uint16 +} + +func (e dXSError) SequenceId() uint16 { return e.seqId } +func (_ dXSError) BadId() uint32 { return 0 } +func (_ dXSError) Error() string { return "dummy X server error reply" } + +func newDummyXServerReplier() func([]byte) []byte { + // register xgb error & event replies + NewErrorFuncs[255] = func(buf []byte) Error { + return dXSError{Get16(buf[2:])} + } + NewEventFuncs[128&127] = func(buf []byte) Event { + return dXSEvent{} + } + + // sequence number generator + seqId := uint16(1) + incrementSequenceId := func() { + // this has to be the same algorithm as in (*Conn).generateSeqIds + if seqId == uint16((1<<16)-1) { + seqId = 0 + } else { + seqId++ + } + } + return func(request []byte) []byte { + res := make([]byte, 32) + switch string(request) { + case "event": + res[0] = 128 + return res + case "error": + res[0] = 0 // error + res[1] = 255 // error function + default: + res[0] = 1 // reply + } + Put16(res[2:], seqId) // sequence number + incrementSequenceId() + if string(request) == "noreply" { + return nil + } + return res + } +} diff --git a/testingTools_test.go b/testingTools_test.go new file mode 100644 index 0000000..518b326 --- /dev/null +++ b/testingTools_test.go @@ -0,0 +1,350 @@ +package xgb + +import ( + "bytes" + "errors" + "fmt" + "io" + "reflect" + "sync" + "testing" + "time" +) + +func TestLeaks(t *testing.T) { + lm := leaksMonitor("lm") + if lgrs := lm.leakingGoroutines(); len(lgrs) != 0 { + t.Errorf("leakingGoroutines returned %d leaking goroutines, want 0", len(lgrs)) + } + + done := make(chan struct{}) + wg := &sync.WaitGroup{} + + wg.Add(1) + go func() { + <-done + wg.Done() + }() + + if lgrs := lm.leakingGoroutines(); len(lgrs) != 1 { + t.Errorf("leakingGoroutines returned %d leaking goroutines, want 1", len(lgrs)) + } + + wg.Add(1) + go func() { + <-done + wg.Done() + }() + + if lgrs := lm.leakingGoroutines(); len(lgrs) != 2 { + t.Errorf("leakingGoroutines returned %d leaking goroutines, want 2", len(lgrs)) + } + + close(done) + wg.Wait() + + if lgrs := lm.leakingGoroutines(); len(lgrs) != 0 { + t.Errorf("leakingGoroutines returned %d leaking goroutines, want 0", len(lgrs)) + } + + lm.checkTesting(t) + //TODO multiple leak monitors with report ignore tests +} + +func TestDummyNetConn(t *testing.T) { + ioStatesPairGenerator := func(writeStates, readStates []string) []func() (*dNC, error) { + writeSetters := map[string]func(*dNC) error{ + "lock": (*dNC).WriteLock, + "error": (*dNC).WriteError, + "success": (*dNC).WriteSuccess, + } + readSetters := map[string]func(*dNC) error{ + "lock": (*dNC).ReadLock, + "error": (*dNC).ReadError, + "success": (*dNC).ReadSuccess, + } + + res := []func() (*dNC, error){} + for _, writeState := range writeStates { + writeState, writeSetter := writeState, writeSetters[writeState] + if writeSetter == nil { + panic("unknown write state: " + writeState) + continue + } + for _, readState := range readStates { + readState, readSetter := readState, readSetters[readState] + if readSetter == nil { + panic("unknown read state: " + readState) + continue + } + res = append(res, func() (*dNC, error) { + + // loopback server + s := newDummyNetConn("w:"+writeState+";r:"+readState, func(b []byte) []byte { return b }) + + if err := readSetter(s); err != nil { + s.Close() + return nil, errors.New("set read " + readState + " error: " + err.Error()) + } + + if err := writeSetter(s); err != nil { + s.Close() + return nil, errors.New("set write " + writeState + " error: " + err.Error()) + } + + return s, nil + }) + } + } + return res + } + + timeout := 10 * time.Millisecond + wantResponse := func(action func(*dNC) error, want, block error) func(*dNC) error { + return func(s *dNC) error { + actionResult := make(chan error) + timedOut := make(chan struct{}) + go func() { + err := action(s) + select { + case <-timedOut: + if err != block { + t.Errorf("after unblocking, action result=%v, want %v", err, block) + } + case actionResult <- err: + } + }() + select { + case err := <-actionResult: + if err != want { + return errors.New(fmt.Sprintf("action result=%v, want %v", err, want)) + } + case <-time.After(timeout): + close(timedOut) + return errors.New(fmt.Sprintf("action did not respond for %v, result want %v", timeout, want)) + } + return nil + } + } + wantBlock := func(action func(*dNC) error, unblock error) func(*dNC) error { + return func(s *dNC) error { + actionResult := make(chan error) + timedOut := make(chan struct{}) + go func() { + err := action(s) + select { + case <-timedOut: + if err != unblock { + t.Errorf("after unblocking, action result=%v, want %v", err, unblock) + } + case actionResult <- err: + } + }() + select { + case err := <-actionResult: + return errors.New(fmt.Sprintf("action result=%v, want to be blocked", err)) + case <-time.After(timeout): + close(timedOut) + } + return nil + } + } + write := func(b string) func(*dNC) error { + return func(s *dNC) error { + n, err := s.Write([]byte(b)) + if err == nil && n != len(b) { + return errors.New("Write returned nil error, but not everything was written") + } + return err + } + } + read := func(b string) func(*dNC) error { + return func(s *dNC) error { + r := make([]byte, len(b)) + n, err := s.Read(r) + if err == nil { + if n != len(b) { + return errors.New("Read returned nil error, but not everything was read") + } + if !reflect.DeepEqual(r, []byte(b)) { + return errors.New("Read=\"" + string(r) + "\", want \"" + string(b) + "\"") + } + } + return err + } + } + + testCases := []struct { + description string + servers []func() (*dNC, error) + actions []func(*dNC) error // actions per server + }{ + {"close,close", + ioStatesPairGenerator( + []string{"lock", "error", "success"}, + []string{"lock", "error", "success"}, + ), + []func(*dNC) error{ + wantResponse((*dNC).Close, nil, dNCErrClosed), + wantResponse((*dNC).Close, dNCErrClosed, dNCErrClosed), + }, + }, + {"write,close,write", + ioStatesPairGenerator( + []string{"lock"}, + []string{"lock", "error", "success"}, + ), + []func(*dNC) error{ + wantBlock(write(""), dNCErrClosed), + wantResponse((*dNC).Close, nil, dNCErrClosed), + wantResponse(write(""), dNCErrClosed, dNCErrClosed), + }, + }, + {"write,close,write", + ioStatesPairGenerator( + []string{"error"}, + []string{"lock", "error", "success"}, + ), + []func(*dNC) error{ + wantResponse(write(""), dNCErrWrite, dNCErrClosed), + wantResponse((*dNC).Close, nil, dNCErrClosed), + wantResponse(write(""), dNCErrClosed, dNCErrClosed), + }, + }, + {"write,close,write", + ioStatesPairGenerator( + []string{"success"}, + []string{"lock", "error", "success"}, + ), + []func(*dNC) error{ + wantResponse(write(""), nil, dNCErrClosed), + wantResponse((*dNC).Close, nil, dNCErrClosed), + wantResponse(write(""), dNCErrClosed, dNCErrClosed), + }, + }, + {"read,close,read", + ioStatesPairGenerator( + []string{"lock", "error", "success"}, + []string{"lock", "error", "success"}, + ), + []func(*dNC) error{ + wantBlock(read(""), io.EOF), + wantResponse((*dNC).Close, nil, dNCErrClosed), + wantResponse(read(""), io.EOF, io.EOF), + }, + }, + {"write,read", + ioStatesPairGenerator( + []string{"lock"}, + []string{"lock", "error", "success"}, + ), + []func(*dNC) error{ + wantBlock(write("1"), dNCErrClosed), + wantBlock(read("1"), io.EOF), + }, + }, + {"write,read", + ioStatesPairGenerator( + []string{"error"}, + []string{"lock", "error", "success"}, + ), + []func(*dNC) error{ + wantResponse(write("1"), dNCErrWrite, dNCErrClosed), + wantBlock(read("1"), io.EOF), + }, + }, + {"write,read", + ioStatesPairGenerator( + []string{"success"}, + []string{"lock"}, + ), + []func(*dNC) error{ + wantResponse(write("1"), nil, dNCErrClosed), + wantBlock(read("1"), io.EOF), + }, + }, + {"write,read", + ioStatesPairGenerator( + []string{"success"}, + []string{"error"}, + ), + []func(*dNC) error{ + wantResponse(write("1"), nil, dNCErrClosed), + wantResponse(read("1"), dNCErrRead, io.EOF), + }, + }, + {"write,read", + ioStatesPairGenerator( + []string{"success"}, + []string{"success"}, + ), + []func(*dNC) error{ + wantResponse(write("1"), nil, dNCErrClosed), + wantResponse(read("1"), nil, io.EOF), + }, + }, + } + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + defer leaksMonitor(tc.description).checkTesting(t) + + for _, server := range tc.servers { + s, err := server() + if err != nil { + t.Error(err) + continue + } + if s == nil { + t.Error("nil server in testcase") + continue + } + + t.Run(s.LocalAddr().String(), func(t *testing.T) { + defer leaksMonitor(s.LocalAddr().String()).checkTesting(t) + for _, action := range tc.actions { + if err := action(s); err != nil { + t.Error(err) + break + } + } + s.Close() + }) + } + }) + } +} + +func TestDummyXServerReplier(t *testing.T) { + testCases := [][][2][]byte{ + { + [2][]byte{[]byte("reply"), []byte{1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + [2][]byte{[]byte("eply"), []byte{1, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + [2][]byte{[]byte("ply"), []byte{1, 0, 3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + [2][]byte{[]byte("event"), []byte{128, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + [2][]byte{[]byte("ly"), []byte{1, 0, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + [2][]byte{[]byte("y"), []byte{1, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + [2][]byte{[]byte(""), []byte{1, 0, 6, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + [2][]byte{[]byte("event"), []byte{128, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + [2][]byte{[]byte("reply"), []byte{1, 0, 7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + [2][]byte{[]byte("error"), []byte{0, 255, 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + [2][]byte{[]byte("ply"), []byte{1, 0, 9, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + [2][]byte{[]byte("event"), []byte{128, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + [2][]byte{[]byte("ly"), []byte{1, 0, 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + [2][]byte{[]byte("noreply"), nil}, + [2][]byte{[]byte("error"), []byte{0, 255, 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + [2][]byte{[]byte("noreply"), nil}, + [2][]byte{[]byte(""), []byte{1, 0, 14, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, + }, + } + + for tci, tc := range testCases { + replier := newDummyXServerReplier() + for ai, ioPair := range tc { + in, want := ioPair[0], ioPair[1] + if out := replier(in); !bytes.Equal(out, want) { + t.Errorf("testCase %d, action %d, replier(%s) = %v, want %v", tci, ai, string(in), out, want) + break + } + } + } +} diff --git a/xcmisc/xcmisc.go b/xcmisc/xcmisc.go index 1778057..93dd20f 100644 --- a/xcmisc/xcmisc.go +++ b/xcmisc/xcmisc.go @@ -4,9 +4,9 @@ package xcmisc // This file is automatically generated from xc_misc.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the XC-MISC extension. diff --git a/xevie/xevie.go b/xevie/xevie.go index 180312b..f0ddd31 100644 --- a/xevie/xevie.go +++ b/xevie/xevie.go @@ -4,9 +4,9 @@ package xevie // This file is automatically generated from xevie.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the XEVIE extension. diff --git a/xf86dri/xf86dri.go b/xf86dri/xf86dri.go index 51c9322..bc67e9b 100644 --- a/xf86dri/xf86dri.go +++ b/xf86dri/xf86dri.go @@ -4,9 +4,9 @@ package xf86dri // This file is automatically generated from xf86dri.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the XFree86-DRI extension. diff --git a/xf86vidmode/xf86vidmode.go b/xf86vidmode/xf86vidmode.go index 2829fd6..6bd3232 100644 --- a/xf86vidmode/xf86vidmode.go +++ b/xf86vidmode/xf86vidmode.go @@ -4,9 +4,9 @@ package xf86vidmode // This file is automatically generated from xf86vidmode.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the XFree86-VidModeExtension extension. diff --git a/xfixes/xfixes.go b/xfixes/xfixes.go index 0f9e4b0..5751527 100644 --- a/xfixes/xfixes.go +++ b/xfixes/xfixes.go @@ -4,11 +4,11 @@ package xfixes // This file is automatically generated from xfixes.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/render" - "github.com/BurntSushi/xgb/shape" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/render" + "github.com/jezek/xgb/shape" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the XFIXES extension. @@ -60,7 +60,8 @@ type Conn struct { xidChan chan xid seqChan chan uint16 reqChan chan *request - closing chan chan struct{} + doneSend chan struct{} + doneRead chan struct{} // ExtLock is a lock used whenever new extensions are initialized. // It should not be used. It is exported for use in the extension @@ -75,11 +76,13 @@ type Conn struct { // NewConn creates a new connection instance. It initializes locks, data // structures, and performs the initial handshake. (The code for the handshake // has been relegated to conn.go.) +// It is up to user to close connection with Close() method to finish all unfinished requests and clean up spawned goroutines. +// If the connection unexpectedly closes itself and WaitForEvent() returns "nil, nil", everything is cleaned by that moment, but nothing bad happens if you call Close() after. func NewConn() (*Conn, error) { return NewConnDisplay("") } -// NewConnDisplay is just like NewConn, but allows a specific DISPLAY +// NewConnDisplay is just like NewConn (see closing instructions), but allows a specific DISPLAY // string to be used. // If 'display' is empty it will be taken from os.Getenv("DISPLAY"). // @@ -89,55 +92,60 @@ func NewConn() (*Conn, error) { // NewConn("hostname:2.1") -> net.Dial("tcp", "", "hostname:6002") // NewConn("tcp/hostname:1.0") -> net.Dial("tcp", "", "hostname:6001") func NewConnDisplay(display string) (*Conn, error) { - conn := &Conn{} + c := &Conn{} // First connect. This reads authority, checks DISPLAY environment // variable, and loads the initial Setup info. - err := conn.connect(display) + err := c.connect(display) if err != nil { return nil, err } - return postNewConn(conn) + return postNewConn(c) } -// NewConnDisplay is just like NewConn, but allows a specific net.Conn +// NewConnNet is just like NewConn (see closing instructions), but allows a specific net.Conn // to be used. func NewConnNet(netConn net.Conn) (*Conn, error) { - conn := &Conn{} + c := &Conn{} // First connect. This reads authority, checks DISPLAY environment // variable, and loads the initial Setup info. - err := conn.connectNet(netConn) + err := c.connectNet(netConn) if err != nil { return nil, err } - return postNewConn(conn) + return postNewConn(c) } -func postNewConn(conn *Conn) (*Conn, error) { - conn.Extensions = make(map[string]byte) +func postNewConn(c *Conn) (*Conn, error) { + c.Extensions = make(map[string]byte) - conn.cookieChan = make(chan *Cookie, cookieBuffer) - conn.xidChan = make(chan xid, xidBuffer) - conn.seqChan = make(chan uint16, seqBuffer) - conn.reqChan = make(chan *request, reqBuffer) - conn.eventChan = make(chan eventOrError, eventBuffer) - conn.closing = make(chan chan struct{}, 1) + c.cookieChan = make(chan *Cookie, cookieBuffer) + c.xidChan = make(chan xid, xidBuffer) + c.seqChan = make(chan uint16, seqBuffer) + c.reqChan = make(chan *request, reqBuffer) + c.eventChan = make(chan eventOrError, eventBuffer) + c.doneSend = make(chan struct{}) + c.doneRead = make(chan struct{}) - go conn.generateXIds() - go conn.generateSeqIds() - go conn.sendRequests() - go conn.readResponses() + go c.generateXIds() + go c.generateSeqIds() + go c.sendRequests() + go c.readResponses() - return conn, nil + return c, nil } // Close gracefully closes the connection to the X server. +// When everything is cleaned up, the WaitForEvent method will return (nil, nil) func (c *Conn) Close() { - close(c.reqChan) + select { + case c.reqChan <- nil: + case <-c.doneSend: + } } // Event is an interface that can contain any of the events returned by the @@ -196,8 +204,12 @@ type eventOrError interface{} // If you need identifiers, use the appropriate constructor. // e.g., For a window id, use xproto.NewWindowId. For // a new pixmap id, use xproto.NewPixmapId. And so on. +// Returns (0, io.EOF) when the connection is closed. func (c *Conn) NewId() (uint32, error) { - xid := <-c.xidChan + xid, ok := <-c.xidChan + if !ok { + return 0, io.EOF + } if xid.err != nil { return 0, xid.err } @@ -217,8 +229,8 @@ type xid struct { // This needs to be updated to use the XC Misc extension once we run out of // new ids. // Thanks to libxcb/src/xcb_xid.c. This code is greatly inspired by it. -func (conn *Conn) generateXIds() { - defer close(conn.xidChan) +func (c *Conn) generateXIds() { + defer close(c.xidChan) // This requires some explanation. From the horse's mouth: // "The resource-id-mask contains a single contiguous set of bits (at least @@ -236,23 +248,30 @@ func (conn *Conn) generateXIds() { // 00111000 & 11001000 = 00001000. // And we use that value to increment the last resource id to get a new one. // (And then, of course, we OR it with resource-id-base.) - inc := conn.setupResourceIdMask & -conn.setupResourceIdMask - max := conn.setupResourceIdMask + inc := c.setupResourceIdMask & -c.setupResourceIdMask + max := c.setupResourceIdMask last := uint32(0) for { - // TODO: Use the XC Misc extension to look for released ids. + id := xid{} if last > 0 && last >= max-inc+1 { - conn.xidChan <- xid{ - id: 0, - err: errors.New("There are no more available resource" + - "identifiers."), + // TODO: Use the XC Misc extension to look for released ids. + id = xid{ + id: 0, + err: errors.New("There are no more available resource identifiers."), + } + } else { + last += inc + id = xid{ + id: last | c.setupResourceIdBase, + err: nil, } } - last += inc - conn.xidChan <- xid{ - id: last | conn.setupResourceIdBase, - err: nil, + select { + case c.xidChan <- id: + case <-c.doneSend: + // c.sendRequests is down and since this id is used by requests, we don't need this goroutine running anymore. + return } } } @@ -275,11 +294,16 @@ func (c *Conn) generateSeqIds() { seqid := uint16(1) for { - c.seqChan <- seqid - if seqid == uint16((1<<16)-1) { - seqid = 0 - } else { - seqid++ + select { + case c.seqChan <- seqid: + if seqid == uint16((1<<16)-1) { + seqid = 0 + } else { + seqid++ + } + case <-c.doneSend: + // c.sendRequests is down and since only that function uses sequence ids (via newSequenceId method), we don't need this goroutine running anymore. + return } } } @@ -315,8 +339,19 @@ type request struct { // edits the generated code for the request you want to issue. func (c *Conn) NewRequest(buf []byte, cookie *Cookie) { seq := make(chan struct{}) - c.reqChan <- &request{buf: buf, cookie: cookie, seq: seq} - <-seq + select { + case c.reqChan <- &request{buf: buf, cookie: cookie, seq: seq}: + // request is in buffer + // wait until request is processed or connection is closed + select { + case <-seq: + // request was successfully sent to X server + case <-c.doneSend: + // c.sendRequests is down, your request was not handled + } + case <-c.doneSend: + // c.sendRequests is down, nobody is listening to your requests + } } // sendRequests is run as a single goroutine that takes requests and writes @@ -324,28 +359,45 @@ func (c *Conn) NewRequest(buf []byte, cookie *Cookie) { // It is meant to be run as its own goroutine. func (c *Conn) sendRequests() { defer close(c.cookieChan) + defer c.conn.Close() + defer close(c.doneSend) - for req := range c.reqChan { - // ho there! if the cookie channel is nearly full, force a round - // trip to clear out the cookie buffer. - // Note that we circumvent the request channel, because we're *in* - // the request channel. - if len(c.cookieChan) == cookieBuffer-1 { - if err := c.noop(); err != nil { - // Shut everything down. - break + for { + select { + case req := <-c.reqChan: + if req == nil { + // a request by c.Close() to gracefully exit + // Flush the response reading goroutine. + if err := c.noop(); err != nil { + c.conn.Close() + <-c.doneRead + } + return + } + // ho there! if the cookie channel is nearly full, force a round + // trip to clear out the cookie buffer. + // Note that we circumvent the request channel, because we're *in* + // the request channel. + if len(c.cookieChan) == cookieBuffer-1 { + if err := c.noop(); err != nil { + // Shut everything down. + c.conn.Close() + <-c.doneRead + return + } + } + req.cookie.Sequence = c.newSequenceId() + c.cookieChan <- req.cookie + if err := c.writeBuffer(req.buf); err != nil { + c.conn.Close() + <-c.doneRead + return } + close(req.seq) + case <-c.doneRead: + return } - req.cookie.Sequence = c.newSequenceId() - c.cookieChan <- req.cookie - c.writeBuffer(req.buf) - close(req.seq) } - response := make(chan struct{}) - c.closing <- response - c.noop() // Flush the response reading goroutine, ignore error. - <-response - c.conn.Close() } // noop circumvents the usual request sending goroutines and forces a round @@ -366,9 +418,8 @@ func (c *Conn) writeBuffer(buf []byte) error { if _, err := c.conn.Write(buf); err != nil { Logger.Printf("A write error is unrecoverable: %s", err) return err - } else { - return nil } + return nil } // readResponses is a goroutine that reads events, errors and @@ -382,6 +433,8 @@ func (c *Conn) writeBuffer(buf []byte) error { // Finally, cookies that came "before" this reply are always cleaned up. func (c *Conn) readResponses() { defer close(c.eventChan) + defer c.conn.Close() + defer close(c.doneRead) var ( err Error @@ -390,20 +443,18 @@ func (c *Conn) readResponses() { ) for { - select { - case respond := <-c.closing: - respond <- struct{}{} - return - default: - } - buf := make([]byte, 32) err, seq = nil, 0 if _, err := io.ReadFull(c.conn, buf); err != nil { + select { + case <-c.doneSend: + // gracefully closing + return + default: + } Logger.Printf("A read error is unrecoverable: %s", err) c.eventChan <- err - c.Close() - continue + return } switch buf[0] { case 0: // This is an error @@ -432,8 +483,7 @@ func (c *Conn) readResponses() { if _, err := io.ReadFull(c.conn, biggerBuf[32:]); err != nil { Logger.Printf("A read error is unrecoverable: %s", err) c.eventChan <- err - c.Close() - continue + return } replyBytes = biggerBuf } else { @@ -522,10 +572,14 @@ func processEventOrError(everr eventOrError) (Event, Error) { return ee, nil case Error: return nil, ee + case error: + // c.conn read error + case nil: + // c.eventChan is closed default: Logger.Printf("Invalid event/error type: %T", everr) - return nil, nil } + return nil, nil } // WaitForEvent returns the next event from the server. diff --git a/xgb_test.go b/xgb_test.go new file mode 100644 index 0000000..19ed307 --- /dev/null +++ b/xgb_test.go @@ -0,0 +1,225 @@ +package xgb + +import ( + "errors" + "fmt" + "testing" + "time" +) + +func TestConnOnNonBlockingDummyXServer(t *testing.T) { + timeout := 10 * time.Millisecond + checkedReply := func(wantError bool) func(*Conn) error { + request := "reply" + if wantError { + request = "error" + } + return func(c *Conn) error { + cookie := c.NewCookie(true, true) + c.NewRequest([]byte(request), cookie) + _, err := cookie.Reply() + if wantError && err == nil { + return errors.New(fmt.Sprintf("checked request \"%v\" with reply resulted in nil error, want some error", request)) + } + if !wantError && err != nil { + return errors.New(fmt.Sprintf("checked request \"%v\" with reply resulted in error %v, want nil error", request, err)) + } + return nil + } + } + checkedNoreply := func(wantError bool) func(*Conn) error { + request := "noreply" + if wantError { + request = "error" + } + return func(c *Conn) error { + cookie := c.NewCookie(true, false) + c.NewRequest([]byte(request), cookie) + err := cookie.Check() + if wantError && err == nil { + return errors.New(fmt.Sprintf("checked request \"%v\" with no reply resulted in nil error, want some error", request)) + } + if !wantError && err != nil { + return errors.New(fmt.Sprintf("checked request \"%v\" with no reply resulted in error %v, want nil error", request, err)) + } + return nil + } + } + uncheckedReply := func(wantError bool) func(*Conn) error { + request := "reply" + if wantError { + request = "error" + } + return func(c *Conn) error { + cookie := c.NewCookie(false, true) + c.NewRequest([]byte(request), cookie) + _, err := cookie.Reply() + if err != nil { + return errors.New(fmt.Sprintf("unchecked request \"%v\" with reply resulted in %v, want nil", request, err)) + } + return nil + } + } + uncheckedNoreply := func(wantError bool) func(*Conn) error { + request := "noreply" + if wantError { + request = "error" + } + return func(c *Conn) error { + cookie := c.NewCookie(false, false) + c.NewRequest([]byte(request), cookie) + return nil + } + } + event := func() func(*Conn) error { + return func(c *Conn) error { + _, err := c.conn.Write([]byte("event")) + if err != nil { + return errors.New(fmt.Sprintf("asked dummy server to send event, but resulted in error: %v\n", err)) + } + return err + } + } + waitEvent := func(wantError bool) func(*Conn) error { + return func(c *Conn) error { + _, err := c.WaitForEvent() + if wantError && err == nil { + return errors.New(fmt.Sprintf("wait for event resulted in nil error, want some error")) + } + if !wantError && err != nil { + return errors.New(fmt.Sprintf("wait for event resulted in error %v, want nil error", err)) + } + return nil + } + } + checkClosed := func(c *Conn) error { + select { + case eoe, ok := <-c.eventChan: + if ok { + return fmt.Errorf("(*Conn).eventChan should be closed, but is not and returns %v", eoe) + } + case <-time.After(timeout): + return fmt.Errorf("(*Conn).eventChan should be closed, but is not and was blocking for %v", timeout) + } + return nil + } + + testCases := []struct { + description string + actions []func(*Conn) error + }{ + {"close", + []func(*Conn) error{}, + }, + {"double close", + []func(*Conn) error{ + func(c *Conn) error { + c.Close() + return nil + }, + }, + }, + {"checked requests with reply", + []func(*Conn) error{ + checkedReply(false), + checkedReply(true), + checkedReply(false), + checkedReply(true), + }, + }, + {"checked requests no reply", + []func(*Conn) error{ + checkedNoreply(false), + checkedNoreply(true), + checkedNoreply(false), + checkedNoreply(true), + }, + }, + {"unchecked requests with reply", + []func(*Conn) error{ + uncheckedReply(false), + uncheckedReply(true), + waitEvent(true), + uncheckedReply(false), + event(), + waitEvent(false), + }, + }, + {"unchecked requests no reply", + []func(*Conn) error{ + uncheckedNoreply(false), + uncheckedNoreply(true), + waitEvent(true), + uncheckedNoreply(false), + event(), + waitEvent(false), + }, + }, + {"close with pending requests", + []func(*Conn) error{ + func(c *Conn) error { + c.conn.(*dNC).ReadLock() + defer c.conn.(*dNC).ReadUnlock() + c.NewRequest([]byte("reply"), c.NewCookie(false, true)) + c.Close() + return nil + }, + checkClosed, + }, + }, + {"unexpected conn close", + []func(*Conn) error{ + func(c *Conn) error { + c.conn.Close() + if ev, err := c.WaitForEvent(); ev != nil || err != nil { + return fmt.Errorf("WaitForEvent() = (%v, %v), want (nil, nil)", ev, err) + } + return nil + }, + checkClosed, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + sclm := leaksMonitor("after server close, before testcase exit") + defer sclm.checkTesting(t) + + s := newDummyNetConn("dummyX", newDummyXServerReplier()) + defer s.Close() + + c, err := postNewConn(&Conn{conn: s}) + if err != nil { + t.Errorf("connect to dummy server error: %v", err) + return + } + + defer leaksMonitor("after actions end", sclm).checkTesting(t) + + for _, action := range tc.actions { + if err := action(c); err != nil { + t.Error(err) + break + } + } + + recovered := false + func() { + defer func() { + if err := recover(); err != nil { + t.Errorf("(*Conn).Close() panic recover: %v", err) + recovered = true + } + }() + + c.Close() + }() + if !recovered { + if err := checkClosed(c); err != nil { + t.Error(err) + } + } + + }) + } +} diff --git a/xgbgen/context.go b/xgbgen/context.go index f18fd67..755cf6e 100644 --- a/xgbgen/context.go +++ b/xgbgen/context.go @@ -64,10 +64,10 @@ func (c *Context) Morph(xmlBytes []byte) { // Write imports. We always need to import at least xgb. // We also need to import xproto if it's an extension. c.Putln("import (") - c.Putln("\"github.com/BurntSushi/xgb\"") + c.Putln("\"github.com/jezek/xgb\"") c.Putln("") if c.protocol.isExt() { - c.Putln("\"github.com/BurntSushi/xgb/xproto\"") + c.Putln("\"github.com/jezek/xgb/xproto\"") } sort.Sort(Protocols(c.protocol.Imports)) @@ -76,7 +76,7 @@ func (c *Context) Morph(xmlBytes []byte) { if imp.Name == "xproto" { continue } - c.Putln("\"github.com/BurntSushi/xgb/%s\"", imp.Name) + c.Putln("\"github.com/jezek/xgb/%s\"", imp.Name) } c.Putln(")") c.Putln("") diff --git a/xinerama/xinerama.go b/xinerama/xinerama.go index ec97406..580387c 100644 --- a/xinerama/xinerama.go +++ b/xinerama/xinerama.go @@ -4,9 +4,9 @@ package xinerama // This file is automatically generated from xinerama.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the XINERAMA extension. diff --git a/xprint/xprint.go b/xprint/xprint.go index d692aed..97e06e1 100644 --- a/xprint/xprint.go +++ b/xprint/xprint.go @@ -4,9 +4,9 @@ package xprint // This file is automatically generated from xprint.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the XpExtension extension. diff --git a/xproto/xproto.go b/xproto/xproto.go index 716c49b..8d7e51b 100644 --- a/xproto/xproto.go +++ b/xproto/xproto.go @@ -4,7 +4,7 @@ package xproto // This file is automatically generated from xproto.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" ) // Setup parses the setup bytes retrieved when diff --git a/xproto/xproto_test.go b/xproto/xproto_test.go index a5bec71..f8080bf 100644 --- a/xproto/xproto_test.go +++ b/xproto/xproto_test.go @@ -26,7 +26,7 @@ import ( "testing" "time" - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" ) // The X connection used throughout testing. diff --git a/xselinux/xselinux.go b/xselinux/xselinux.go index 1afcc10..68d55ea 100644 --- a/xselinux/xselinux.go +++ b/xselinux/xselinux.go @@ -4,9 +4,9 @@ package xselinux // This file is automatically generated from xselinux.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the SELinux extension. diff --git a/xtest/xtest.go b/xtest/xtest.go index 182760e..c11cfc6 100644 --- a/xtest/xtest.go +++ b/xtest/xtest.go @@ -4,9 +4,9 @@ package xtest // This file is automatically generated from xtest.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the XTEST extension. @@ -4,10 +4,10 @@ package xv // This file is automatically generated from xv.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/shm" - "github.com/BurntSushi/xgb/xproto" + "github.com/jezek/xgb/shm" + "github.com/jezek/xgb/xproto" ) // Init must be called before using the XVideo extension. diff --git a/xvmc/xvmc.go b/xvmc/xvmc.go index b943fa0..3f61416 100644 --- a/xvmc/xvmc.go +++ b/xvmc/xvmc.go @@ -4,10 +4,10 @@ package xvmc // This file is automatically generated from xvmc.xml. Edit at your peril! import ( - "github.com/BurntSushi/xgb" + "github.com/jezek/xgb" - "github.com/BurntSushi/xgb/xproto" - "github.com/BurntSushi/xgb/xv" + "github.com/jezek/xgb/xproto" + "github.com/jezek/xgb/xv" ) // Init must be called before using the XVideo-MotionCompensation extension. |