SIGSEGV upon incoming UDP packet

[yoursunny]

When YaNFD version e12c57105546d5e8bb1c005fdecfb5f150c4c298 receives an incoming UDP packet, it crashes with the following error:

  INFO[0003] [UnicastUDPTransport, FaceID=0, RemoteURI=udp4://192.168.94.0:6363, LocalURI=udp4://192.168.94.1:6363] state: Down -> Up
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x5f97c2]

goroutine 61 [running]:
github.com/Link512/stealthpool.(*Pool).tryPop(0x0)
        /go/pkg/mod/github.com/zjkmxy/stealthpool@v0.2.1/pool.go:128 +0x42
github.com/Link512/stealthpool.(*Pool).Get(0x0)
        /go/pkg/mod/github.com/zjkmxy/stealthpool@v0.2.1/pool.go:76 +0x4b
github.com/named-data/YaNFD/face.(*NDNLPLinkService).handleIncomingFrame(0xc00061e3c0, {0xc00039a500, 0x3d, 0x0?})
        /app/face/ndnlp-link-service.go:383 +0x36
github.com/named-data/YaNFD/face.(*UDPListener).Run(0xc0004b0240)
        /app/face/udp-listener.go:105 +0x5f1
created by github.com/named-data/YaNFD/executor.(*YaNFD).Start
        /app/executor/yanfd.go:203 +0x17aa

The cause is:

• In NDNLPLinkService type, l.stealthPool is initialized in Run method and used in handleIncomingFrame method.
• In UDPListener type, handleIncomingFrame is called before Run.

[pulsejet]

@zjkmxy any idea why handleIncomingFrame is called before Run?

[yoursunny]

Reversing the order won't help because Run is in a new goroutine. You need to create l.stealthPool in MakeNDNLPLinkService so that it's always there.

[zjkmxy]

@zjkmxy any idea why handleIncomingFrame is called before Run?

UDP face is created upon reception of a packet. Thus that packet must be submitted to the face. In current implementation, Run() is run as a coroutine, and there is no way to wait until the creation of l.stealthPool without refactoring the interface.

[zjkmxy]

Reversing the order won't help because Run is in a new goroutine. You need to create l.stealthPool in MakeNDNLPLinkService so that it's always there.

Actually they did this at the beginning. But they wanted to defer the destruction of stealthpool. Currently, there is no function such as LinkService.Close(), so they moved the function into Run().

[yoursunny]

You still can defer l.stealthPool.Close() in the Run function.

[pulsejet]

I think a couple of solutions:

1/ Make the stealthpool create on first use. As long as Run() is called at some point, we don't have a memleak.

2/ Get rid of it altogether. It's really meant to reduce just one copy, not sure it makes too big a difference.

[yoursunny]

The stealthPool has nothing to do with "to reduce one copy". It's for reducing GC overhead.

I think you can just use a global sync.Pool.