Skip to content

refactor(packetrelay): remove SetWriteIdleTimeout from PacketListenerRelay#628

Open
fortuna wants to merge 1 commit into
mainfrom
remove-set-write-idle-timeout
Open

refactor(packetrelay): remove SetWriteIdleTimeout from PacketListenerRelay#628
fortuna wants to merge 1 commit into
mainfrom
remove-set-write-idle-timeout

Conversation

@fortuna

@fortuna fortuna commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR simplifies the PacketListenerRelay code.

  • Removes SetWriteIdleTimeout from PacketListenerRelay — the timeout is set once at construction and does not need to be mutable after that
  • Removes the sync.RWMutex and associated lock/unlock calls that existed solely to protect that method
  • Adds two race-condition tests (TestPacketListenerRelaySendCloseRace, TestPacketListenerRelayReceiveCloseRace) verified with -race

Test plan

  • go test -race ./network/packetrelay/... passes

🤖 Generated with Claude Code

…Relay

The timeout is set once at construction via NewPacketRelayFromPacketListener
and does not need to be mutable after that. Removes the method, the
RWMutex it required, and adds race-condition tests for concurrent
SendPacket/Close and ReceivePackets/Close.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jun 23, 2026

Copy link
Copy Markdown

Greptile Summary

This PR simplifies PacketListenerRelay by removing the mutable SetWriteIdleTimeout method and the sync.RWMutex that existed solely to protect it, since the timeout is now fixed at construction time. Two new race-condition tests are added to verify that concurrent send/close and receive/close scenarios are handled correctly under the Go race detector.

  • SetWriteIdleTimeout and the sync.RWMutex on PacketListenerRelay are removed; NewAssociation now reads listener and writeIdleTimeout directly since they are immutable after construction.
  • TestPacketListenerRelaySendCloseRace and TestPacketListenerRelayReceiveCloseRace are added with a new blockingPacketConn helper, replacing the now-irrelevant SetWriteIdleTimeout tests.
  • The deprecation comment in network/packet_listener_proxy.go still points to PacketListenerRelay.SetWriteIdleTimeout, which no longer exists.

Confidence Score: 4/5

Safe to merge after updating the stale deprecation comment in packet_listener_proxy.go.

The lock removal is correct — the two struct fields are now immutable after construction, so NewAssociation has no concurrent writer to guard against. The new race tests exercise the important send/close and receive/close interleavings. The only issue is the deprecation doc comment in network/packet_listener_proxy.go that still references the now-removed SetWriteIdleTimeout, which would produce a broken godoc link and mislead users attempting to follow the migration path.

network/packet_listener_proxy.go — its WithPacketListenerWriteIdleTimeout deprecation comment references the removed method.

Important Files Changed

Filename Overview
network/packetrelay/packet_listener_relay.go Removes SetWriteIdleTimeout and the sync.RWMutex that protected it; fields are now immutable after construction so no synchronization is needed in NewAssociation.
network/packetrelay/packet_listener_relay_test.go Replaces the old SetWriteIdleTimeout tests with two targeted race-condition tests (send/close and receive/close), and adds the necessary blockingPacketConn and connListener helpers.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant PacketListenerRelay
    participant packetListenerAssociation
    participant net.PacketConn

    Caller->>PacketListenerRelay: NewPacketRelayFromPacketListener(pl, timeout)
    Note over PacketListenerRelay: listener and writeIdleTimeout<br/>set once, immutable after return

    Caller->>PacketListenerRelay: NewAssociation()
    PacketListenerRelay->>net.PacketConn: listener.ListenPacket(ctx)
    net.PacketConn-->>PacketListenerRelay: packetConn
    PacketListenerRelay->>packetListenerAssociation: refreshDeadline()
    packetListenerAssociation->>net.PacketConn: SetDeadline(now + writeIdleTimeout)
    PacketListenerRelay-->>Caller: sender, receiver

    par Send path
        Caller->>packetListenerAssociation: sender.SendPacket(p, dst)
        packetListenerAssociation->>net.PacketConn: SetDeadline (refresh)
        packetListenerAssociation->>net.PacketConn: WriteTo(p, dst)
    and Close path
        Caller->>packetListenerAssociation: sender.Close()
        packetListenerAssociation->>net.PacketConn: Close()
        Note over packetListenerAssociation: close() guarded by sync.Mutex
    end

    net.PacketConn-->>packetListenerAssociation: ReadFrom returns error on close
    packetListenerAssociation-->>Caller: receiver.ReceivePackets returns
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant PacketListenerRelay
    participant packetListenerAssociation
    participant net.PacketConn

    Caller->>PacketListenerRelay: NewPacketRelayFromPacketListener(pl, timeout)
    Note over PacketListenerRelay: listener and writeIdleTimeout<br/>set once, immutable after return

    Caller->>PacketListenerRelay: NewAssociation()
    PacketListenerRelay->>net.PacketConn: listener.ListenPacket(ctx)
    net.PacketConn-->>PacketListenerRelay: packetConn
    PacketListenerRelay->>packetListenerAssociation: refreshDeadline()
    packetListenerAssociation->>net.PacketConn: SetDeadline(now + writeIdleTimeout)
    PacketListenerRelay-->>Caller: sender, receiver

    par Send path
        Caller->>packetListenerAssociation: sender.SendPacket(p, dst)
        packetListenerAssociation->>net.PacketConn: SetDeadline (refresh)
        packetListenerAssociation->>net.PacketConn: WriteTo(p, dst)
    and Close path
        Caller->>packetListenerAssociation: sender.Close()
        packetListenerAssociation->>net.PacketConn: Close()
        Note over packetListenerAssociation: close() guarded by sync.Mutex
    end

    net.PacketConn-->>packetListenerAssociation: ReadFrom returns error on close
    packetListenerAssociation-->>Caller: receiver.ReceivePackets returns
Loading

Reviews (1): Last reviewed commit: "refactor(packetrelay): remove SetWriteId..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant