named-data / YaNFD

Yet another Named Data Networking Forwarding Daemon
https://pkg.go.dev/github.com/named-data/YaNFD
MIT License
12 stars 10 forks source link

Concurrent map write for pitTokenMap #38

Closed pulsejet closed 2 years ago

pulsejet commented 2 years ago

There is some concurrent map write in Pit. Maybe because expirationPitLoop runs in a separate goroutine

https://github.com/named-data/YaNFD/blob/1b359373e000d5550da5e465ce346a79128d7f24/table/pit-cs-tree.go#L77

and this is not okay for the map deletion in RemoveInterest.

https://github.com/named-data/YaNFD/blob/1b359373e000d5550da5e465ce346a79128d7f24/table/pit-cs-tree.go#L99 https://github.com/named-data/YaNFD/blob/1b359373e000d5550da5e465ce346a79128d7f24/table/pit-cs-tree.go#L175

fatal error: concurrent map writes
goroutine 14 [running]:                                                                                                                                     runtime.throw({0x82a5ae?, 0x60?})
/snap/go/9605/src/runtime/panic.go:992 +0x71 fp=0xc00033c6d0 sp=0xc00033c6a0 pc=0x439a71                                                            runtime.mapdelete_fast32(0xc00033c778?, 0x1?, 0x1b5840bb)
/snap/go/9605/src/runtime/map_fast32.go:282 +0x2aa fp=0xc00033c718 sp=0xc00033c6d0 pc=0x41520a
github.com/named-data/YaNFD/table.(*PitCsTree).RemoveInterest(0xc000198e40?, {0x8cf988?, 0xc00340dc20?})
/home/vpatil/yanfd/table/pit-cs-tree.go:175 +0xee fp=0xc00033c740 sp=0xc00033c718 pc=0x58788e
github.com/named-data/YaNFD/table.(*PitCsTree).expirationPitLoop(0xc000198e40)
/home/vpatil/yanfd/table/pit-cs-tree.go:99 +0x196 fp=0xc00033c7c8 sp=0xc00033c740 pc=0x587096                                                       github.com/named-data/YaNFD/table.NewPitCS.func1()
/home/vpatil/yanfd/table/pit-cs-tree.go:77 +0x26 fp=0xc00033c7e0 sp=0xc00033c7c8 pc=0x586ec6                                                        runtime.goexit()
/snap/go/9605/src/runtime/asm_amd64.s:1571 +0x1 fp=0xc00033c7e8 sp=0xc00033c7e0 pc=0x469d01
created by github.com/named-data/YaNFD/table.NewPitCS
        /home/vpatil/yanfd/table/pit-cs-tree.go:77 +0x325

EDIT: sorry the log is a mess somehow

zjkmxy commented 2 years ago

Thank you for reporting this. Let me figure out what happened.

pulsejet commented 2 years ago

I also got some errors about pitExpiryQueue being nil? Not sure if its the same thing

zjkmxy commented 2 years ago

Besides that, pitExpiryQueue is also used by two coroutines. I think the final solution should be move go pitCs.expirationPitLoop() to the forwarding thread loop.

zjkmxy commented 2 years ago

Hello, @pulsejet . If you have time could you please check if #40 fixes the problem? Thanks. I removed the expirationPitLoop() instead of adding a lock.