Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rclone hang #74

Merged
merged 1 commit into from
Feb 13, 2023
Merged

Fix rclone hang #74

merged 1 commit into from
Feb 13, 2023

Conversation

slifty
Copy link
Contributor

@slifty slifty commented Feb 8, 2023

This PR adds some fairly gnarly code to accommodate a bug in the upstream library that handles implementation of the ssh / sftp protocol's more inner workings.

There is already an upstream patch which unfortunately has not been merged yet. Once that is merged we can delete this local patch.

Resolves #45

@slifty slifty force-pushed the 45-fix-rclone-hang branch 2 times, most recently from e74b0fb to e1e89a0 Compare February 8, 2023 19:33
There is an open PR in the upstream / ssh2 library related to properly
closing SFTP channels when a `CHANNEL_EOF` message is received[1].
Unfortunately that PR has not yet been merged.

Until that happens we need to handle the signal ourselves.
Unfortunately this requires us to access "private" attributes
(JavaScript doesn't support the concept of private attributes, so we are
able to do this...). This is, of course a terrible and horrible idea and
all of our tooling gets very upset about it.  As a result I had to
disable the tooling for the relevant line.

Once the PR is merged in upstream we should upgrade to it and delete the
contents of this commit.

Issue #45 rclone hangs after completion

[1] mscdex/ssh2#1111
Copy link
Member

@cecilia-donnelly cecilia-donnelly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @slifty! This all makes sense to me and I appreciate the detailed comment.

Copy link
Contributor

@kfogel kfogel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done. This change is about as clean as it possibly can be, and it explains itself well.

@slifty slifty merged commit 1e276b6 into main Feb 13, 2023
@slifty slifty deleted the 45-fix-rclone-hang branch February 13, 2023 17:36
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.

rclone hangs after completion
3 participants