fix(sftp): handle SSH channels concurrently to fix 40s freeze with gvfs/Nautilus

[Xgabi86]

Fixes pelican-dev/panel#2399

Root Cause

gvfs (used by GNOME Nautilus and KDE Dolphin) opens two separate SSH connections to the SFTP server via SSH ControlMaster multiplexing: a command_connection and a data_connection. Both share the same underlying TCP connection, meaning Wings receives two channel open requests on the same ssh.ServerConn.

The issue is in AcceptInbound: the for ch := range chans loop calls c.Handle() synchronously, blocking on the first channel's entire SFTP session. The second channel requested by gvfs is queued in chans but never dequeued — Wings is stuck inside Handle() for the first channel. After ~40 seconds, gvfs times out and falls back to a degraded mode, logging Setting up data connection failed.

FileZilla is unaffected because it only opens a single SFTP channel per connection.

Fix

Wrap c.Handle() in a goroutine so each SSH channel is served concurrently, allowing Wings to process multiple channels on the same connection simultaneously.

Investigation

Diagnosed via:

• tshark packet capture confirming a precise ~40s silence after SSH_MSG_CHANNEL_OPEN_CONFIRMATION
• strace on gvfsd-sftp confirming two ssh processes spawned with -oControlMaster auto
• gvfs 1.60.0 source code (gvfsbackendsftp.c) confirming the dual-connection architecture
• Wings source code confirming the synchronous channel handling

Summary by CodeRabbit

• Refactor
  • SFTP server now processes multiple channel connections concurrently for improved performance and responsiveness when handling simultaneous connections.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 59c7e829-9088-4372-b225-8289e01bd927

📥 Commits

Reviewing files that changed from the base of the PR and between 58c5717 and 08417f0.

📒 Files selected for processing (1)

• sftp/server.go

📜 Recent review details

🔇 Additional comments (1)

sftp/server.go (1)

152-157: LGTM!

---

📝 Walkthrough

Walkthrough

In SFTPServer.AcceptInbound, the call to c.Handle(sconn, srv, channel) is changed from a synchronous, blocking call whose error was returned to the caller, to a goroutine that logs any resulting error, allowing the accept loop to process additional channels concurrently.

Changes

Concurrent SFTP Channel Handling

Layer / File(s)	Summary
AcceptInbound goroutine dispatch sftp/server.go	c.Handle is now launched in a goroutine; errors are logged rather than returned, unblocking AcceptInbound from waiting on each channel handler before accepting the next.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Possibly related issues

• #2399 (SFTP connection takes 30-90 seconds to establish with GNOME Nautilus): Nautilus likely opens multiple SFTP channels during connection setup; the previous synchronous c.Handle would block processing of each subsequent channel until the prior one completed, explaining the multi-second delay. The goroutine dispatch directly addresses this bottleneck.

Poem

🐇 Hop, hop, no more waiting in line,
Each channel now runs in its own little vine.
The goroutine leaps while the loop carries on,
No blocking, no stalling from dusk until dawn.
Nautilus flies — the slow tunnel is gone! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: making SSH channel handling concurrent to fix the freeze issue reported in the linked issue.
Linked Issues check	✅ Passed	The PR directly addresses issue #2399 by implementing concurrent channel handling to resolve the 30-90 second connection delay with GNOME Nautilus.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing the concurrent channel handling issue; no unrelated modifications were introduced.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[parkervcp]

Let me do some investigation for this.