Skip to content

Context propagation improvements after noctx linter fix #4976

@JonathanOppenheimer

Description

@JonathanOppenheimer

the following is AI-generated by claude:


PR #4975 resolved the noctx linter violations by threading context.Context through the network, peer, NAT, and RPC layers. Several reviewers on the original PR (#4163) identified additional improvements that were explicitly deferred as follow-ups.

Follow-up Items

1. Replace signal.Notify with signal.NotifyContext in app startup

Files: main/main.go, app/app.go

Currently, main/main.go passes context.Background() to both app.New() and app.Run(), while signal handling is done separately via the os/signal.Notify channel pattern. These could be unified by using signal.NotifyContext to create a context that auto-cancels on SIGTERM/SIGINT, then threading that context through the entire application lifecycle. This would give all downstream consumers (network, peers, listeners) a context that naturally cancels on shutdown.

Originally suggested by @maru-ava, agreed upon by @joshua-kim as a follow-up in PR #4163 review.

2. Thread context through subprocess.NewCmd

File: vms/rpcchainvm/runtime/subprocess/non_linux_stopper.go

NewCmd currently hardcodes context.Background() to satisfy the linter:

func NewCmd(path string, args ...string) *exec.Cmd {
    return exec.CommandContext(context.Background(), path, args...)
}

The signature should accept a context.Context parameter so callers can provide a meaningful context. Care must be taken to ensure subprocess contexts are appropriately scoped — as noted by @StephenButtolph, a caller's context being cancelled must not kill a subprocess that should outlive the call (see process_runtime.go's use of context.WithoutCancel as a reference pattern).

Originally identified by @maru-ava in PR #4163 review.

3. Document context lifecycle expectations in the network package

Files: network/peer/peer.go, network/peer/network.go, network/network.go

PR #4975 threads context from peer.Start() through long-lived goroutines (readMessages, writeMessages, sendNetworkMessages) and into Network.Track() and Network.Disconnected(). As discussed by @aaronbuchwald, this changes the implicit contract — previously these operations used no context (or context.Background()), and now a caller-provided context flows through long-lived async paths.

While the root context passed into avalanchego is never cancelled (making this safe in practice), the interface now permits callers to pass contexts that could be cancelled. Adding documentation to the Network interface and peer.Start() clarifying the expected context lifecycle (i.e., the context should not be cancelled before shutdown) would prevent subtle misuse by external consumers.

Metadata

Metadata

Assignees

No one assigned

    Labels

    cleanupCode quality improvementhelp wantedLooking for someone to address this

    Type

    Projects

    Status

    No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions