search

esiqveland / notify

notify is a go dbus implementation for delivering desktop notifications over dbus
BSD 3-Clause "New" or "Revised" License
70 stars 14 forks source link

Fix racy shutdown logic #11

Closed Merovius closed 2 years ago

Merovius commented 2 years ago

There are two issues with the shutdown logic:

  1. If Close() is called before the eventLoop is scheduled, the WaitGroup counter is still 0 and Close does not wait for the eventLoop to finish. That's a race condition
  2. If Close() is called more than once, it panics, as the code tries to write to a closed channel. It is generally good practice to make Close idempotent, to allow the defer x.Close(); /* … */ return x.Close() idiom, which is a convenient way to guarantee both that Close is called and that the error is checked.

This commit fixes both and factors the shutdown logic into a new type for readability and ease of understanding.

esiqveland commented 2 years ago

Hey, thank you. It is nicer that Close() is idempotent for the user.