scroot / gopacket

Automatically exported from code.google.com/p/gopacket
BSD 3-Clause "New" or "Revised" License
0 stars 1 forks source link

Example arpscan.go leaks goroutine #20

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
AFAIK arpscan.go:writeARP() leaks a running goroutine while iterating over the 
result of ips() and returning due to an error condition. See 

https://code.google.com/p/gopacket/source/browse/examples/arpscan/arpscan.go#149

Original issue reported on code.google.com by lukas.l...@gmail.com on 25 Jun 2014 at 2:46

GoogleCodeExporter commented 9 years ago
Given that main calls scan() only once, then exits, I'm not sure I'd 
particularly call it "leaking" a goroutine, since it exits within 10 seconds 
and all goroutines then die.

That said, I'd happily accept a patch that wouldn't leak the goroutine were the 
scan call in main looped :)

Going to mark WontFix for now, but please feel free to push back if you feel 
that I've misunderstood the issue you pointed out.  Thanks for the report!

Original comment by gsconn...@gmail.com on 26 Jun 2014 at 5:55

GoogleCodeExporter commented 9 years ago
The point being entirely that this is example code in a google-signed project 
and me being new to Go and getting confused: I was looking at the code and 
thought "oh look, a simple pattern for garbage collected iterators in go" until 
someone on #go-nuts confirmed that the goroutine created by ips() would live 
forever.

You may want to place a comment on #149 in writeARP() to stop people from 
wondering if this is clever or a bug.

Original comment by lukas.l...@gmail.com on 26 Jun 2014 at 6:08

GoogleCodeExporter commented 9 years ago
Great idea.

I've added a comment to the readARP function, see 
https://code.google.com/p/gopacket/source/browse/examples/arpscan/arpscan.go?spe
c=svna3a121d10b80daec124b134718311288bbe23ecb&r=a3a121d10b80daec124b134718311288
bbe23ecb#101

Original comment by gconnell@google.com on 26 Jun 2014 at 3:56

GoogleCodeExporter commented 9 years ago
Whoops, I commented on the wrong goroutine leak ;)

I've fixed ips() to return a slice in 
https://code.google.com/p/gopacket/source/detail?r=724ba5139a6c4c7e115844941d5db
279320bc08a

Original comment by gconnell@google.com on 26 Jun 2014 at 4:02