fiorix / go-smpp

SMPP 3.4 Protocol for the Go programming language
MIT License
218 stars 135 forks source link

client.BindFunc blocks on client.Read (resource leak) #79

Closed michalm86 closed 5 years ago

michalm86 commented 5 years ago

Using:

tx := &smpp.Transceiver{
  ...
}
tx.Bind()

on unstable connection leads to memory leak (sorry for line numbers - issue encountered on old github.com/zpas-lab/go-smpp fork):

github.com/zpas-lab/go-smpp/smpp.(*client).Read(0xc4201264d0, 0xc420e62f58, 0xc40000ac38, 0xc420bdbde8, 0x0)
        github.com/zpas-lab/go-smpp/smpp/client.go:170 +0xf8
github.com/zpas-lab/go-smpp/smpp.(*Transmitter).handlePDU(0xc4202d3b68, 0xc4205248a0)
        github.com/zpas-lab/go-smpp/smpp/transmitter.go:103 +0x44
created by github.com/zpas-lab/go-smpp/smpp.(*Transceiver).bindFunc
        github.com/zpas-lab/go-smpp/smpp/transceiver.go:74 +0x43a

For current code would be:

client.go:225
transmitter.go:118
transceiver.go:80

This happens because transceiver calls go client.Bind(), in which:

  1. l.130-138: connection is set up
  2. l.139: client.BindFunc is called, which is transceiver.bindFunc injected, which calls go transmitter.handlePDU, which calls client.Read, which waits on client.inbox channel)
  3. l.147: if for whatever reason err is returned, break in l.153 is called, which means there is no send to client.inbox. close(client.inbox) is called nowhere, so transceiver.bindFunc waits forever

If this scenario repeats, we end up with more and more goroutines (transceiver.bindFunc -> transmitter.handlePDU -> client.Read) waiting on the client.inbox channel.

fiorix commented 5 years ago

Good catch. I'd be happy to review and merge PRs.

fiorix commented 5 years ago

Merged #80