BurntSushi / 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
489 stars 75 forks source link

Stop goroutines when closing a connection #39

Open stolyaroleh opened 7 years ago

stolyaroleh commented 7 years ago

Hello, @BurntSushi. I would like to use xgb in one of my projects. It's a service that runs in background and talks to the X server on behalf of all currently logged in users. Since it maintains a pool of X11 connections that can die at any moment (for example, when an interactive session stops on logout), I wanted to make sure that xgb will not panic/leave garbage behind when this happens.

This pull request attempts to fix #32, by introducing a sync.WaitGroup and blocking in Close() until all 4 goroutines, spawned by NewConnNet die. I tested it by creating and closing 100 000 connections - decreased RAM usage from at least 700 Mb to ~25 Mb.

Additionally, I use xgb.Conn.done (previously xgb.Conn.closing, a chan struct{}) to broadcast that all goroutines need to stop (receive on closed channel does not block).

Finally, I broadcastDone on unrecoverable read errors in readResponses instead of xgb.Conn.Close(). Since readResponses contains an infinite loop, and previously would just continue after Close() (line 406, xgb.go), it would attempt to Close() again in the next iteration, causing a panic (closing an already closed channel).

I don't have much experience with xgb or X11 in general and would greatly appreciate if you had a look at this, in case I missed something.