Skip to content

Commit 3dcb3e9

Browse files
zeripath6543lafriks
authored
Second attempt at preventing zombies (#16326)
* Second attempt at preventing zombies * Ensure that the pipes are closed in ssh.go * Ensure that a cancellable context is passed up in cmd/* http requests * Make cmd.fail return properly so defers are obeyed * Ensure that something is sent to stdout in case of blocks here Signed-off-by: Andrew Thornton <[email protected]> * placate lint Signed-off-by: Andrew Thornton <[email protected]> * placate lint 2 Signed-off-by: Andrew Thornton <[email protected]> * placate lint 3 Signed-off-by: Andrew Thornton <[email protected]> * fixup Signed-off-by: Andrew Thornton <[email protected]> * Apply suggestions from code review Co-authored-by: 6543 <[email protected]> Co-authored-by: Lauris BH <[email protected]>
1 parent ee43d70 commit 3dcb3e9

21 files changed

+229
-143
lines changed

‎cmd/cmd.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,13 @@
77
package cmd
88

99
import (
10+
"context"
1011
"errors"
1112
"fmt"
13+
"os"
14+
"os/signal"
1215
"strings"
16+
"syscall"
1317

1418
"code.gitea.io/gitea/models"
1519
"code.gitea.io/gitea/modules/setting"
@@ -66,3 +70,25 @@ func initDBDisableConsole(disableConsole bool) error {
6670
}
6771
return nil
6872
}
73+
74+
func installSignals() (context.Context, context.CancelFunc) {
75+
ctx, cancel := context.WithCancel(context.Background())
76+
go func() {
77+
// install notify
78+
signalChannel := make(chan os.Signal, 1)
79+
80+
signal.Notify(
81+
signalChannel,
82+
syscall.SIGINT,
83+
syscall.SIGTERM,
84+
)
85+
select {
86+
case <-signalChannel:
87+
case <-ctx.Done():
88+
}
89+
cancel()
90+
signal.Reset()
91+
}()
92+
93+
return ctx, cancel
94+
}

‎cmd/hook.go

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -152,17 +152,18 @@ func runHookPreReceive(c *cli.Context) error {
152152
if os.Getenv(models.EnvIsInternal) == "true" {
153153
return nil
154154
}
155+
ctx, cancel := installSignals()
156+
defer cancel()
155157

156158
setup("hooks/pre-receive.log", c.Bool("debug"))
157159

158160
if len(os.Getenv("SSH_ORIGINAL_COMMAND")) == 0 {
159161
if setting.OnlyAllowPushIfGiteaEnvironmentSet {
160-
fail(`Rejecting changes as Gitea environment not set.
162+
return fail(`Rejecting changes as Gitea environment not set.
161163
If you are pushing over SSH you must push with a key managed by
162164
Gitea or set your environment appropriately.`, "")
163-
} else {
164-
return nil
165165
}
166+
return nil
166167
}
167168

168169
// the environment is set by serv command
@@ -235,14 +236,14 @@ Gitea or set your environment appropriately.`, "")
235236
hookOptions.OldCommitIDs = oldCommitIDs
236237
hookOptions.NewCommitIDs = newCommitIDs
237238
hookOptions.RefFullNames = refFullNames
238-
statusCode, msg := private.HookPreReceive(username, reponame, hookOptions)
239+
statusCode, msg := private.HookPreReceive(ctx, username, reponame, hookOptions)
239240
switch statusCode {
240241
case http.StatusOK:
241242
// no-op
242243
case http.StatusInternalServerError:
243-
fail("Internal Server Error", msg)
244+
return fail("Internal Server Error", msg)
244245
default:
245-
fail(msg, "")
246+
return fail(msg, "")
246247
}
247248
count = 0
248249
lastline = 0
@@ -263,12 +264,12 @@ Gitea or set your environment appropriately.`, "")
263264

264265
fmt.Fprintf(out, " Checking %d references\n", count)
265266

266-
statusCode, msg := private.HookPreReceive(username, reponame, hookOptions)
267+
statusCode, msg := private.HookPreReceive(ctx, username, reponame, hookOptions)
267268
switch statusCode {
268269
case http.StatusInternalServerError:
269-
fail("Internal Server Error", msg)
270+
return fail("Internal Server Error", msg)
270271
case http.StatusForbidden:
271-
fail(msg, "")
272+
return fail(msg, "")
272273
}
273274
} else if lastline > 0 {
274275
fmt.Fprintf(out, "\n")
@@ -285,8 +286,11 @@ func runHookUpdate(c *cli.Context) error {
285286
}
286287

287288
func runHookPostReceive(c *cli.Context) error {
289+
ctx, cancel := installSignals()
290+
defer cancel()
291+
288292
// First of all run update-server-info no matter what
289-
if _, err := git.NewCommand("update-server-info").Run(); err != nil {
293+
if _, err := git.NewCommand("update-server-info").SetParentContext(ctx).Run(); err != nil {
290294
return fmt.Errorf("Failed to call 'git update-server-info': %v", err)
291295
}
292296

@@ -299,12 +303,11 @@ func runHookPostReceive(c *cli.Context) error {
299303

300304
if len(os.Getenv("SSH_ORIGINAL_COMMAND")) == 0 {
301305
if setting.OnlyAllowPushIfGiteaEnvironmentSet {
302-
fail(`Rejecting changes as Gitea environment not set.
306+
return fail(`Rejecting changes as Gitea environment not set.
303307
If you are pushing over SSH you must push with a key managed by
304308
Gitea or set your environment appropriately.`, "")
305-
} else {
306-
return nil
307309
}
310+
return nil
308311
}
309312

310313
var out io.Writer
@@ -371,11 +374,11 @@ Gitea or set your environment appropriately.`, "")
371374
hookOptions.OldCommitIDs = oldCommitIDs
372375
hookOptions.NewCommitIDs = newCommitIDs
373376
hookOptions.RefFullNames = refFullNames
374-
resp, err := private.HookPostReceive(repoUser, repoName, hookOptions)
377+
resp, err := private.HookPostReceive(ctx, repoUser, repoName, hookOptions)
375378
if resp == nil {
376379
_ = dWriter.Close()
377380
hookPrintResults(results)
378-
fail("Internal Server Error", err)
381+
return fail("Internal Server Error", err)
379382
}
380383
wasEmpty = wasEmpty || resp.RepoWasEmpty
381384
results = append(results, resp.Results...)
@@ -386,9 +389,9 @@ Gitea or set your environment appropriately.`, "")
386389
if count == 0 {
387390
if wasEmpty && masterPushed {
388391
// We need to tell the repo to reset the default branch to master
389-
err := private.SetDefaultBranch(repoUser, repoName, "master")
392+
err := private.SetDefaultBranch(ctx, repoUser, repoName, "master")
390393
if err != nil {
391-
fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err)
394+
return fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err)
392395
}
393396
}
394397
fmt.Fprintf(out, "Processed %d references in total\n", total)
@@ -404,11 +407,11 @@ Gitea or set your environment appropriately.`, "")
404407

405408
fmt.Fprintf(out, " Processing %d references\n", count)
406409

407-
resp, err := private.HookPostReceive(repoUser, repoName, hookOptions)
410+
resp, err := private.HookPostReceive(ctx, repoUser, repoName, hookOptions)
408411
if resp == nil {
409412
_ = dWriter.Close()
410413
hookPrintResults(results)
411-
fail("Internal Server Error", err)
414+
return fail("Internal Server Error", err)
412415
}
413416
wasEmpty = wasEmpty || resp.RepoWasEmpty
414417
results = append(results, resp.Results...)
@@ -417,9 +420,9 @@ Gitea or set your environment appropriately.`, "")
417420

418421
if wasEmpty && masterPushed {
419422
// We need to tell the repo to reset the default branch to master
420-
err := private.SetDefaultBranch(repoUser, repoName, "master")
423+
err := private.SetDefaultBranch(ctx, repoUser, repoName, "master")
421424
if err != nil {
422-
fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err)
425+
return fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err)
423426
}
424427
}
425428
_ = dWriter.Close()

‎cmd/keys.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,12 @@ func runKeys(c *cli.Context) error {
6262
return errors.New("No key type and content provided")
6363
}
6464

65+
ctx, cancel := installSignals()
66+
defer cancel()
67+
6568
setup("keys.log", false)
6669

67-
authorizedString, err := private.AuthorizedPublicKeyByContent(content)
70+
authorizedString, err := private.AuthorizedPublicKeyByContent(ctx, content)
6871
if err != nil {
6972
return err
7073
}

‎cmd/mailer.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ import (
1414
)
1515

1616
func runSendMail(c *cli.Context) error {
17+
ctx, cancel := installSignals()
18+
defer cancel()
19+
1720
setting.NewContext()
1821

1922
if err := argsSet(c, "title"); err != nil {
@@ -39,7 +42,7 @@ func runSendMail(c *cli.Context) error {
3942
}
4043
}
4144

42-
status, message := private.SendEmail(subject, body, nil)
45+
status, message := private.SendEmail(ctx, subject, body, nil)
4346
if status != http.StatusOK {
4447
fmt.Printf("error: %s\n", message)
4548
return nil

‎cmd/manager.go

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -236,10 +236,13 @@ func runRemoveLogger(c *cli.Context) error {
236236
group = log.DEFAULT
237237
}
238238
name := c.Args().First()
239-
statusCode, msg := private.RemoveLogger(group, name)
239+
ctx, cancel := installSignals()
240+
defer cancel()
241+
242+
statusCode, msg := private.RemoveLogger(ctx, group, name)
240243
switch statusCode {
241244
case http.StatusInternalServerError:
242-
fail("InternalServerError", msg)
245+
return fail("InternalServerError", msg)
243246
}
244247

245248
fmt.Fprintln(os.Stdout, msg)
@@ -371,82 +374,103 @@ func commonAddLogger(c *cli.Context, mode string, vals map[string]interface{}) e
371374
if c.IsSet("name") {
372375
name = c.String("name")
373376
}
374-
statusCode, msg := private.AddLogger(group, name, mode, vals)
377+
ctx, cancel := installSignals()
378+
defer cancel()
379+
380+
statusCode, msg := private.AddLogger(ctx, group, name, mode, vals)
375381
switch statusCode {
376382
case http.StatusInternalServerError:
377-
fail("InternalServerError", msg)
383+
return fail("InternalServerError", msg)
378384
}
379385

380386
fmt.Fprintln(os.Stdout, msg)
381387
return nil
382388
}
383389

384390
func runShutdown(c *cli.Context) error {
391+
ctx, cancel := installSignals()
392+
defer cancel()
393+
385394
setup("manager", c.Bool("debug"))
386-
statusCode, msg := private.Shutdown()
395+
statusCode, msg := private.Shutdown(ctx)
387396
switch statusCode {
388397
case http.StatusInternalServerError:
389-
fail("InternalServerError", msg)
398+
return fail("InternalServerError", msg)
390399
}
391400

392401
fmt.Fprintln(os.Stdout, msg)
393402
return nil
394403
}
395404

396405
func runRestart(c *cli.Context) error {
406+
ctx, cancel := installSignals()
407+
defer cancel()
408+
397409
setup("manager", c.Bool("debug"))
398-
statusCode, msg := private.Restart()
410+
statusCode, msg := private.Restart(ctx)
399411
switch statusCode {
400412
case http.StatusInternalServerError:
401-
fail("InternalServerError", msg)
413+
return fail("InternalServerError", msg)
402414
}
403415

404416
fmt.Fprintln(os.Stdout, msg)
405417
return nil
406418
}
407419

408420
func runFlushQueues(c *cli.Context) error {
421+
ctx, cancel := installSignals()
422+
defer cancel()
423+
409424
setup("manager", c.Bool("debug"))
410-
statusCode, msg := private.FlushQueues(c.Duration("timeout"), c.Bool("non-blocking"))
425+
statusCode, msg := private.FlushQueues(ctx, c.Duration("timeout"), c.Bool("non-blocking"))
411426
switch statusCode {
412427
case http.StatusInternalServerError:
413-
fail("InternalServerError", msg)
428+
return fail("InternalServerError", msg)
414429
}
415430

416431
fmt.Fprintln(os.Stdout, msg)
417432
return nil
418433
}
419434

420435
func runPauseLogging(c *cli.Context) error {
436+
ctx, cancel := installSignals()
437+
defer cancel()
438+
421439
setup("manager", c.Bool("debug"))
422-
statusCode, msg := private.PauseLogging()
440+
statusCode, msg := private.PauseLogging(ctx)
423441
switch statusCode {
424442
case http.StatusInternalServerError:
425-
fail("InternalServerError", msg)
443+
return fail("InternalServerError", msg)
426444
}
427445

428446
fmt.Fprintln(os.Stdout, msg)
429447
return nil
430448
}
431449

432450
func runResumeLogging(c *cli.Context) error {
451+
ctx, cancel := installSignals()
452+
defer cancel()
453+
433454
setup("manager", c.Bool("debug"))
434-
statusCode, msg := private.ResumeLogging()
455+
statusCode, msg := private.ResumeLogging(ctx)
435456
switch statusCode {
436457
case http.StatusInternalServerError:
437-
fail("InternalServerError", msg)
458+
return fail("InternalServerError", msg)
438459
}
439460

440461
fmt.Fprintln(os.Stdout, msg)
441462
return nil
442463
}
443464

444465
func runReleaseReopenLogging(c *cli.Context) error {
466+
ctx, cancel := installSignals()
467+
defer cancel()
468+
445469
setup("manager", c.Bool("debug"))
446-
statusCode, msg := private.ReleaseReopenLogging()
470+
statusCode, msg := private.ReleaseReopenLogging(ctx)
447471
switch statusCode {
448472
case http.StatusInternalServerError:
449-
fail("InternalServerError", msg)
473+
return fail("InternalServerError", msg)
450474
}
451475

452476
fmt.Fprintln(os.Stdout, msg)

‎cmd/restore_repo.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,24 @@ var CmdRestoreRepository = cli.Command{
4040
cli.StringFlag{
4141
Name: "units",
4242
Value: "",
43-
Usage: `Which items will be restored, one or more units should be separated as comma.
43+
Usage: `Which items will be restored, one or more units should be separated as comma.
4444
wiki, issues, labels, releases, release_assets, milestones, pull_requests, comments are allowed. Empty means all units.`,
4545
},
4646
},
4747
}
4848

49-
func runRestoreRepository(ctx *cli.Context) error {
49+
func runRestoreRepository(c *cli.Context) error {
50+
ctx, cancel := installSignals()
51+
defer cancel()
52+
5053
setting.NewContext()
5154

5255
statusCode, errStr := private.RestoreRepo(
53-
ctx.String("repo_dir"),
54-
ctx.String("owner_name"),
55-
ctx.String("repo_name"),
56-
ctx.StringSlice("units"),
56+
ctx,
57+
c.String("repo_dir"),
58+
c.String("owner_name"),
59+
c.String("repo_name"),
60+
c.StringSlice("units"),
5761
)
5862
if statusCode == http.StatusOK {
5963
return nil

0 commit comments

Comments
 (0)