jezek / xgb

The X Go Binding is a low-level API to communicate with the X server. It is modeled on XCB and supports many X extensions.
Other
130 stars 13 forks source link

Log & shutdown when the eventChan buffer fills up #24

Open hhvn-uk opened 3 months ago

hhvn-uk commented 3 months ago

If the event channel is full, all requests to the X server will get stuck. In theory the channel could be read from, but the sequential event loop structure typically used by xgb programs will not do so.

See also: #23

hhvn-uk commented 3 months ago

If the event channel is full, all requests to the X server will get stuck. In theory the channel could be read from, but the sequential event loop structure typically used by xgb programs will not do so.

I went off and thought about this and realized it is just an excuse because I didn't think of a way to log only once without discarding the event. Here's a better way of doing it I think:

diff --git a/xgb.go b/xgb.go
index d605da6..7a5b42e 100644
--- a/xgb.go
+++ b/xgb.go
@@ -422,6 +422,19 @@ func (c *Conn) writeBuffer(buf []byte) error {
        return nil
 }

+// writeEvent logs a warning if the buffer is full before writing an event
+func (c *Conn) writeEvent(e eventOrError) {
+       select {
+       case c.eventChan <- e:
+               return
+       default:
+               Logger.Printf("%s\n%s",
+                       "The event channel for the connection is full.",
+                       "Are you calling X.WaitForEvent or X.PollForEvent frequently enough?")
+               c.eventChan <- e // blocks here in case the channel is read later
+       }
+}
+
 // readResponses is a goroutine that reads events, errors and
 // replies off the wire.
 // When an event is read, it is always added to the event channel.
@@ -453,7 +466,7 @@ func (c *Conn) readResponses() {
                        default:
                        }
                        Logger.Printf("A read error is unrecoverable: %s", err)
-                       c.eventChan <- err
+                       c.writeEvent(err)
                        return
                }
                switch buf[0] {
@@ -482,7 +495,7 @@ func (c *Conn) readResponses() {
                                copy(biggerBuf[:32], buf)
                                if _, err := io.ReadFull(c.conn, biggerBuf[32:]); err != nil {
                                        Logger.Printf("A read error is unrecoverable: %s", err)
-                                       c.eventChan <- err
+                                       c.writeEvent(err)
                                        return
                                }
                                replyBytes = biggerBuf
@@ -504,7 +517,7 @@ func (c *Conn) readResponses() {
                                        "for event with number %d.", evNum)
                                continue
                        }
-                       c.eventChan <- newEventFun(buf)
+                       c.writeEvent(newEventFun(buf))
                        continue
                }

@@ -524,7 +537,7 @@ func (c *Conn) readResponses() {
                                        if cookie.errorChan != nil {
                                                cookie.errorChan <- err
                                        } else { // asynchronous processing
-                                               c.eventChan <- err
+                                               c.writeEvent(err)
                                                // if this is an unchecked reply, ping the cookie too
                                                if cookie.pingChan != nil {
                                                        cookie.pingChan <- true

If you agree I'll force push this instead.