fixes to irc/socket.go

* fix a race condition: a call to `Write` does not spawn a writer goroutine
  if the trylock is held, so `BlockingWrite` must check for fresh data after
  releasing the trylock
* streamline some close/finalize logic
This commit is contained in:
Shivaram Lingamneni 2018-11-28 00:24:44 -05:00
parent a0bf548fc5
commit c8cf0befc6
1 changed files with 24 additions and 9 deletions

View File

@ -134,6 +134,7 @@ func (socket *Socket) Write(data []byte) (err error) {
prospectiveLen := socket.totalLength + len(data) prospectiveLen := socket.totalLength + len(data)
if prospectiveLen > socket.maxSendQBytes { if prospectiveLen > socket.maxSendQBytes {
socket.sendQExceeded = true socket.sendQExceeded = true
socket.closed = true
err = errSendQExceeded err = errSendQExceeded
} else { } else {
socket.buffers = append(socket.buffers, data) socket.buffers = append(socket.buffers, data)
@ -161,6 +162,13 @@ func (socket *Socket) BlockingWrite(data []byte) (err error) {
return return
} }
// after releasing the semaphore, we must check for fresh data, same as `send`
defer func() {
if socket.readyToWrite() {
socket.wakeWriter()
}
}()
// blocking acquire of the trylock // blocking acquire of the trylock
socket.writerSemaphore.Acquire() socket.writerSemaphore.Acquire()
defer socket.writerSemaphore.Release() defer socket.writerSemaphore.Release()
@ -206,7 +214,7 @@ func (socket *Socket) readyToWrite() bool {
socket.Lock() socket.Lock()
defer socket.Unlock() defer socket.Unlock()
// on the first time observing socket.closed, we still have to write socket.finalData // on the first time observing socket.closed, we still have to write socket.finalData
return !socket.finalized && (socket.totalLength > 0 || socket.closed || socket.sendQExceeded) return !socket.finalized && (socket.totalLength > 0 || socket.closed)
} }
// send actually writes messages to socket.Conn; it may block // send actually writes messages to socket.Conn; it may block
@ -238,19 +246,20 @@ func (socket *Socket) performWrite() (closed bool) {
buffers := socket.buffers buffers := socket.buffers
socket.buffers = nil socket.buffers = nil
socket.totalLength = 0 socket.totalLength = 0
closed = socket.closed
socket.Unlock() socket.Unlock()
// on Linux, the runtime will optimize this into a single writev(2) call: var err error
_, err := (*net.Buffers)(&buffers).WriteTo(socket.conn) if !closed && len(buffers) > 0 {
// on Linux, the runtime will optimize this into a single writev(2) call:
_, err = (*net.Buffers)(&buffers).WriteTo(socket.conn)
}
socket.Lock() closed = closed || err != nil
shouldClose := (err != nil) || socket.closed || socket.sendQExceeded if closed {
socket.Unlock()
if shouldClose {
socket.finalize() socket.finalize()
} }
return shouldClose return
} }
// mark closed and send final data. you must be holding the semaphore to call this: // mark closed and send final data. you must be holding the semaphore to call this:
@ -258,12 +267,18 @@ func (socket *Socket) finalize() {
// mark the socket closed (if someone hasn't already), then write error lines // mark the socket closed (if someone hasn't already), then write error lines
socket.Lock() socket.Lock()
socket.closed = true socket.closed = true
finalized := socket.finalized
socket.finalized = true socket.finalized = true
finalData := socket.finalData finalData := socket.finalData
if socket.sendQExceeded { if socket.sendQExceeded {
finalData = "\r\nERROR :SendQ Exceeded\r\n" finalData = "\r\nERROR :SendQ Exceeded\r\n"
} }
socket.Unlock() socket.Unlock()
if finalized {
return
}
if finalData != "" { if finalData != "" {
socket.conn.Write([]byte(finalData)) socket.conn.Write([]byte(finalData))
} }