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: CopyRemotePassThru blocked on writing to error channel #56

Merged
merged 3 commits into from
Apr 13, 2022

Conversation

262nos
Copy link
Contributor

@262nos 262nos commented Mar 18, 2022

While trying to download a file from an remote endpoint, I gave the wrong flags to os.Openfile(dstFileName, os.O_CREATE, 0644), thus creating a lock in my code.

Digging in the code, I saw that the scp client was blocking on a write to errCh in CopyFromRemotePassThru. Adding the wg.Done() unblocked the situation.

Maybe related to #54 and #55

Also, all errors seem to be written twice on errCh as the error is persisted as a variable.

corneliu added 2 commits March 18, 2022 11:03

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@bramvdbogaerde
Copy link
Owner

Thanks for your contribution!

I noticed that you recently pushed another change. Do you want me to leave this PR open in case you want to push more or can I merge it already?

@262nos
Copy link
Contributor Author

262nos commented Mar 18, 2022

I think you can merge it.

For the second commit I didn't have time to add a test for it, but it should work as intended for now.

If I'll have some more time I'll dig through the library some more, but not in the following days.

Thank you.

@bramvdbogaerde bramvdbogaerde merged commit 4c4cfd8 into bramvdbogaerde:master Apr 13, 2022
mudler referenced this pull request in kairos-io/provider-kairos Feb 7, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
|
[github.com/bramvdbogaerde/go-scp](https://togithub.com/bramvdbogaerde/go-scp)
| require | patch | `v1.2.0` -> `v1.2.1` |

---

### Release Notes

<details>
<summary>bramvdbogaerde/go-scp</summary>

###
[`v1.2.1`](https://togithub.com/bramvdbogaerde/go-scp/releases/tag/v1.2.1):
Version 1.2.1

[Compare
Source](https://togithub.com/bramvdbogaerde/go-scp/compare/v1.2.0...v1.2.1)

#### What's Changed

- Some code style fixes by
[@&#8203;dshemin](https://togithub.com/dshemin) in
[https://github.com/bramvdbogaerde/go-scp/pull/53](https://togithub.com/bramvdbogaerde/go-scp/pull/53)
- fix: CopyRemotePassThru blocked on writing to error channel by
[@&#8203;262nos](https://togithub.com/262nos) in
[https://github.com/bramvdbogaerde/go-scp/pull/56](https://togithub.com/bramvdbogaerde/go-scp/pull/56)
- fix: Data race in CopyPassThru by
[@&#8203;mafredri](https://togithub.com/mafredri) in
[https://github.com/bramvdbogaerde/go-scp/pull/66](https://togithub.com/bramvdbogaerde/go-scp/pull/66)

#### New Contributors

- [@&#8203;262nos](https://togithub.com/262nos) made their first
contribution in
[https://github.com/bramvdbogaerde/go-scp/pull/56](https://togithub.com/bramvdbogaerde/go-scp/pull/56)
- [@&#8203;mafredri](https://togithub.com/mafredri) made their first
contribution in
[https://github.com/bramvdbogaerde/go-scp/pull/66](https://togithub.com/bramvdbogaerde/go-scp/pull/66)

**Full Changelog**:
bramvdbogaerde/go-scp@v1.2.0...v1.2.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/kairos-io/provider-kairos).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC44MS4wIiwidXBkYXRlZEluVmVyIjoiMzQuMTA4LjEifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

None yet

2 participants