fix(graceful): prevent WaitGroup panic on Ctrl+C shutdown

[ita004]

What

Guard the wg.Wait() call using sync.Once to avoid
"sync: WaitGroup is reused before previous Wait has returned" panic during shutdown.

Why

Multiple shutdown paths could cause Wait() to be invoked more than once,
which led to a negative WaitGroup counter and panic when pressing Ctrl+C.

How tested

• Built backend locally (make backend)
• Ran ./gitea web and triggered Ctrl+C — server shuts down cleanly (no panic)

Fixes #35559

[lunny]

I don't think I did twice Ctrl+C in #35559

[ita004]

I don't think I did twice Ctrl+C in #35559

yes, I also reproduced the panic with a single Ctrl+C.. the shutdown flow triggers multiple paths that wait on the same WaitGroup, which causes the reuse panic.... this fix ensures wg.Wait() is only called once, so shutdown completes cleanly without the panic.

[wxiaoguang]

This PR's change is just one more patch to the existing messy code (from Restore Graceful Restarting & Socket Activation (#7274)). The root problem is that the "WaitGroup" is abused.

Although @zeripath ever complained that I rewrote his code, but for this case, I think we still need to rewrite the code with a correct design to fix the root problem, just like other complete fixes: logger (#24726), queue (#24505), diff (#19958), etc. It shouldn't add more hacky and fragile patches like #11219.

[ita004]

This PR's change is just one more patch to the existing messy code (from Restore Graceful Restarting & Socket Activation (#7274)). The root problem is that the "WaitGroup" is abused.

Although @zeripath ever complained that I rewrote his code, but for this case, I think we still need to rewrite the code with a correct design to fix the root problem, just like other complete fixes: logger (#24726), queue (#24505), diff (#19958), etc. It shouldn't add more hacky and fragile patches like #11219.

thanks for the detailed feedback, that makes sense...
would like to try introducing a small graceful.Manager helper (to properly gate Add() after shutdown and expose something like Run(fn) bool + Shutdown()) as a cleaner approach.... or do you have any suggestions or preferred direction for how this should be implemented?

[wxiaoguang]

FYI: there are some related fixes like "Refactor graceful manager, fix misused WaitGroup #29738".

[wxiaoguang]

I don't think the current design would work or it can be improved.

doHammer means that it will stop the server immediately without waiting for any running tasks (for example: existing user request connection).

The graceful package's design overall looks problematic and it seems unfixable to me.

[ita004]

thanks for clarifying, i understand what you mean now... that makes sense: doHammer should remain immediate and not wait for any running connections, just forcefully stop the server....

i'll keep this note in mind, and if there’s a future plan to refactor or redesign the graceful manager to address the deeper issues, i'd be happy to participate or help with that whenever it’s appropriate...