CODEWITHSUNDEEP
gogoroutine
Joey Roosing
Joey Roosing

Reputation: 2155

Scanning ports in goroutines

I am currently in the proces of learning go. To do that, I am making a relatively simple portscanner.

The issue I am facing is that it takes a considerable amount of time to scan these ports. The behaviour i am having is that, if i scan ports (defined as an array of int32 (protobuf doesnt support int16), NOT using goroutines works, but is quite slow when scanning more then 5 ports as you can imagine, since it is not running in parallel.

To achieve parallelism, I came up with the following bits of code (explanation + issue comes after code):

//entry point for port scanning
var results []*portscan.ScanResult
//len(splitPorts) is the given string (see benchmark below) chopped up in an int32 slice
ch := make(chan *portscan.ScanResult, len(splitPorts))

var wg sync.WaitGroup
for _, port := range splitPorts {
    connect(ip, port, req.Timeout, ch, &wg)
}
wg.Wait()

for elem := range ch {
    results = append(results, elem)
}

// go routine
func connect(ip string, port, timeout int32, ch chan *portscan.ScanResult, wg *sync.WaitGroup) {
    wg.Add(1)
    go func() {
        res := &portscan.ScanResult{
            Port:   port,
            IsOpen: false,
        }
        conn, err := net.DialTimeout("tcp", fmt.Sprintf("%s:%d", ip, port), time.Duration(timeout)*time.Millisecond)

        if err == nil {
            conn.Close()
            res.IsOpen = true
        }
        ch <- res
        wg.Done()
    }()
}

So protobuf prepared me a struct which looks as follows:

type ScanResult struct {
    Port   int32 `protobuf:"varint,1,opt,name=port" json:"port,omitempty"`
    IsOpen bool  `protobuf:"varint,2,opt,name=isOpen" json:"isOpen,omitempty"`
}

As seen in the first line of the code snippet, I have defined a slice, to hold all results, the idea is that my application scans ports in parallel and when that is done, send the results to whoever is interested.

However, using this code, the program gets stuck.

I run this benchmark to test its performance:

func BenchmarkPortScan(b *testing.B) {
  request := &portscan.ScanPortsRequest{
      Ip:        "62.129.139.214",
      PortRange: "20,21,22,23",
      Timeout:   500,
  }

  svc := newService()

  for i := 0; i < b.N; i++ {
      svc.ScanPorts(nil, request)
  }
}

What is the cause for it to get stuck. Does looking at this code give anything away?

So in short, what I want my end result to be is that each port is scanned in a different go routine, and when they all finished, everything comes together in a resulting slice of ScanResult.

I hope I have been clear and provided enough information for you guys to help me.

Oh, and im especially looking for pointers, and the learning bit, not persee working code samples.

Upvotes: 1

Views: 514

Answers (2)

ain
ain

Reputation: 22749

As @creker wrote, you have to close the channel, othervise the loop which reads from it will be infinite loop. However, I do not agree that just adding close(ch) after the wg.Wait() is right way to go - this would mean that the loop which reads the values from the channel wouldn't start until all ports are scanned (all connect() calls return). I'd say you want to start proccessing the results as soon as they are available. For that you have to restructure your code so that the producer and consumer are different goroutines, something like following

var results []*portscan.ScanResult
ch := make(chan *portscan.ScanResult)

// launch the producer goroutine    
go func() {
   var wg sync.WaitGroup
   wg.Add(len(splitPorts))
   for _, port := range splitPorts {
       go func(port int32) {
          defer wg.Done()
          ch <- connect(ip, port, req.Timeout)
       }(port)
   }
   wg.Wait()
   close(ch)
}()

// consume results
for elem := range ch {
    results = append(results, elem)
}

func connect(ip string, port, timeout int32) *portscan.ScanResult {
    res := &portscan.ScanResult{
            Port:   port,
            IsOpen: false,
    }
    conn, err := net.DialTimeout("tcp", fmt.Sprintf("%s:%d", ip, port), time.Duration(timeout)*time.Millisecond)

    if err == nil {
        conn.Close()
        res.IsOpen = true
    }
    return res
}

Note that now the channel is unbuffered and the connect function doesn't know about the waitgroup or channel, so it is more reusable. You could use buffered channel too, if it turns out that the producers generate data more rapidly than the consumer reads it, but you probaly wouldn't need the buffer to be len(splitPorts) but something smaller.

Another optimisation could be to preallocate the results array as you seem to know the numer of results beforehand (len(splitPorts)) so you do not need to use append.

Upvotes: 2

creker
creker

Reputation: 9570

You need to close the channel after wg.Wait(). Otherwise your range for loop gets stuck.

Other than that your code looks fine.

Upvotes: 2

Related Questions