buildbarn / bb-storage

Storage daemon, capable of storing data for the Remote Execution protocol
Apache License 2.0
137 stars 91 forks source link

Remove panic after graceful shutdown #144

Closed moroten closed 2 years ago

moroten commented 2 years ago

remote-apis-testing shows the following log in https://gitlab.com/remote-apis-testing/remote-apis-testing/-/jobs/2773967861

2022/07/26 20:20:14 Received terminated signal. Initiating graceful shutdown.
2022/07/26 20:20:14 Graceful shutdown complete
panic: Raising the original signal didn't cause us to shut down

Apparently, the panic line was reached before the signal had been handled and the process killed, so just remove it.

EdSchouten commented 2 years ago

Hmmm.... Interesting. It looks like this is the cause for it: https://github.com/golang/go/issues/19326

There's no guarantee the signal gets delivered to the same OS thread, it seems. Let's merge this then.

EdSchouten commented 2 years ago

Oh, wait. Could you maybe add a // comment here along these lines?

This code should not be reached, if it weren't for the fact that process.Signal() does not guarantee that the signal is delivered to the same thread.

moroten commented 2 years ago

Added a comment with a link to the golang ticket.