Call handle.Close method has been blocked

[may11544]

I found a similar issue

#253

I noticed that this problem has been fixed, but it seems to be reproduced in my code, and the version I am using is v1.1.19
My code is as follows

func Tcpdump(device string, filter string, filePath string, fileName string, cancelChan chan struct{}) (err error) {
	var (
		handle *pcap.Handle
		file   *os.File
	)
	handle, err = pcap.OpenLive(device, 65535, true, pcap.BlockForever)
	if err != nil {
		return
	}
	defer func() {
		fmt.Println("begin close...")
		handle.Close()
		fmt.Println("closed...")
	}()
	if err = handle.SetBPFFilter(filter); err != nil {
		return
	}
	packetSource := gopacket.NewPacketSource(handle, handle.LinkType())
	if err = os.MkdirAll(filePath, 0750); err != nil {
		return
	}
	fileFullPath := ""
	if strings.HasSuffix(filePath, "/") {
		fileFullPath = filePath + fileName
	} else {
		fileFullPath = filePath + "/" + fileName
	}
	file, err = os.Create(fileFullPath)
	if err != nil {
		return
	}
	defer func() {
		_ = file.Close()
	}()
	packetWriter := pcapgo.NewWriter(file)
	if err = packetWriter.WriteFileHeader(65535, handle.LinkType()); err != nil {
		return
	}
	for {
		select {
		case packet := <-packetSource.Packets():
			if err = packetWriter.WritePacket(packet.Metadata().CaptureInfo, packet.Data()); err != nil {
				return
			}
		case <-cancelChan:
			return
		}
	}
}

I found that this function cannot end normally, and "begin close..." can be output normally but "closed..." cannot be output

[shiqinfeng1]

I have met the same issue. Are there any solution to bypass such issues ?

[may11544]

I have met the same issue. Are there any solution to bypass such issues ?

I solved the problem by setting a reasonable timeout. As you can see, the timeout in my code is set to pcap.BlockForever. This value is a negative number and causes blocking problems.

handle, err = pcap.OpenLive(device, 65535, true, pcap.BlockForever)

I noticed that Handle's ReadPacketData will be locked, and the synchronization variable is mu.The structure of Handle is as follows

type Handle struct {
	// stop is set to a non-zero value by Handle.Close to signal to
	// getNextBufPtrLocked to stop trying to read packets
	// This must be the first entry to ensure alignment for sync.atomic
	stop uint64
	// cptr is the handle for the actual pcap C object.
	cptr           pcapTPtr
	timeout        time.Duration
	device         string
	deviceIndex    int
	mu             sync.Mutex
	closeMu        sync.Mutex
	nanoSecsFactor int64

	// Since pointers to these objects are passed into a C function, if
	// they're declared locally then the Go compiler thinks they may have
	// escaped into C-land, so it allocates them on the heap.  This causes a
	// huge memory hit, so to handle that we store them here instead.
	pkthdr *pcapPkthdr
	bufptr *uint8
}

func (p *Handle) ReadPacketData() (data []byte, ci gopacket.CaptureInfo, err error) {
	p.mu.Lock()
	err = p.getNextBufPtrLocked(&ci)
	if err == nil {
		data = make([]byte, ci.CaptureLength)
		copy(data, (*(*[1 << 30]byte)(unsafe.Pointer(p.bufptr)))[:])
	}
	p.mu.Unlock()
	if err == NextErrorTimeoutExpired {
		runtime.Gosched()
	}
	return
}

In the Packets() method of PacketSource, a goroutine is opened to call the ReadPacketData method in a loop.
Example in my sample code:
case packet := <-packetSource.Packets():

The source code of this method is as follows

func (p *PacketSource) Packets() chan Packet {
	if p.c == nil {
		p.c = make(chan Packet, 1000)
		go p.packetsToChannel()
	}
	return p.c
}

Note the getNextBufPtrLocked method,When the timeout parameter is less than 0, this method seems to block uniformly.As a result, the lock cannot be released.

// Close closes the underlying pcap handle.
func (p *Handle) Close() {
	p.closeMu.Lock()
	defer p.closeMu.Unlock()

	if !p.isOpen() {
		return
	}

	atomic.StoreUint64(&p.stop, 1)

	// wait for packet reader to stop !!!!!!!!
	p.mu.Lock()
	defer p.mu.Unlock()

	p.pcapClose()
}

Pay attention to these two lines of code

p.mu.Lock()
defer p.mu.Unlock()

[shiqinfeng1]

I got the forked version gopacket/gopacket which soloved this issue by anthor way, but also thanks very much for reply！
#893 (comment)