Fixed stdout/stderr cut off bug when using `RemoteChild::wait_with_output` #104

NobodyXu · 2022-10-29T11:45:14Z

Fixed #103

This PR fixed two bugs:

Fix Fd::as_raw_fd_or_null_fd in native-mux: Disable non-blocking on all fds before passing them to the ssh multiplex server
Fix RemoteChild::wait_with_output: Await on self.wait() after stdout/stderr is fully read in, otherwise the ssh multiplex server would stop writing data to stdout/stderr once self.wait() is done (which drops the connection to the server)

Signed-off-by: Jiahao XU [email protected]

Signed-off-by: Jiahao XU <[email protected]>

… fully read Signed-off-by: Jiahao XU <[email protected]>

jonhoo · 2022-10-29T11:45:20Z

This change is

NobodyXu · 2022-10-29T11:48:26Z

@jonhoo P.S. I would like to squash merge this PR and edit the commit messages since I inverted some commits.

codecov-commenter · 2022-10-29T11:50:27Z

Codecov Report

Merging #104 (908141c) into master (5282f39) will decrease coverage by 0.63%.
The diff coverage is 63.79%.

Additional details and impacted files

Impacted Files	Coverage Δ
src/stdio.rs	`54.63% <0.00%> (-2.36%)`	⬇️
src/native_mux_impl/stdio.rs	`84.09% <72.91%> (-9.39%)`	⬇️
src/child.rs	`89.74% <100.00%> (-0.26%)`	⬇️
src/session.rs	`75.30% <0.00%> (-0.31%)`	⬇️

Signed-off-by: Jiahao XU <[email protected]>

NobodyXu · 2022-10-29T12:59:59Z

@Fuuzetsu Can you test this PR to see if it fixed the bug for you please?

Signed-off-by: Jiahao XU <[email protected]>

src/child.rs

src/native_mux_impl/stdio.rs

Co-authored-by: Jon Gjengset <[email protected]>

Signed-off-by: Jiahao XU <[email protected]>

src/native_mux_impl/stdio.rs

Signed-off-by: Jiahao XU <[email protected]>

Fuuzetsu · 2022-10-31T00:37:59Z

@Fuuzetsu Can you test this PR to see if it fixed the bug for you please?

I checked with 0bb7ad3 and I'm now getting the stdout in full with native-mux. I only tried a couple of times.

NobodyXu · 2022-10-31T00:47:27Z

I checked with 0bb7ad3 and I'm now getting the stdout in full with native-mux. I only tried a couple of times.

Thanks!
If you encountered the problem again when using this PR, feel free to post it here.

src/child.rs

Signed-off-by: Jiahao XU <[email protected]>

src/native_mux_impl/command.rs

Signed-off-by: Jiahao XU <[email protected]>

Do not `set_blocking` on `StdioImpl::inherit` as we do not own them and also do not `set_blocking` on `Stdio` constructed using `FromRawFd::from_raw_fd` Signed-off-by: Jiahao XU <[email protected]>

src/stdio.rs

src/native_mux_impl/stdio.rs

Signed-off-by: Jiahao XU <[email protected]>

src/native_mux_impl/stdio.rs

src/stdio.rs

Signed-off-by: Jiahao XU <[email protected]>

jonhoo · 2022-11-07T00:53:14Z

Oh, can you update the changelog as well?

Signed-off-by: Jiahao XU <[email protected]>

NobodyXu · 2022-11-07T00:58:21Z

Oh, can you update the changelog as well?

I've updated the changelog.

NobodyXu · 2022-11-07T01:11:48Z

P.S. codecov CI seems to fail due to the invalid token:

[2022-11-07T01:01:09.453Z] ['error'] There was an error running the uploader: Error uploading to [https://codecov.io:](https://codecov.io/) Error: There was an error fetching the storage URL during POST: 404 - {'detail': ErrorDetail(string='Unable to locate build via Github Actions API. Please upload with the Codecov repository upload token to resolve issue.', code='not_found')}
Error: Codecov: Failed to properly upload: The process '/home/runner/work/_actions/codecov/codecov-action/v2/dist/codecov' failed with exit code 255

jonhoo · 2022-11-07T01:12:30Z

Oops, I forgot to squash.. Oh well!

jonhoo · 2022-11-07T01:13:18Z

Published in 0.9.8 🎉

jonhoo · 2022-11-07T01:13:30Z

I think that CI failure is probably temporary

NobodyXu · 2022-11-07T01:19:31Z

I think that CI failure is probably temporary

That indeeds seems so since it worked again on master branch.

NobodyXu added 9 commits October 26, 2022 22:30

Add new CI test test_read_large_file_bug

92e673f

Signed-off-by: Jiahao XU <[email protected]>

Impl new fn connects_with_name & use it in test_read_large_file_bug

70a78c1

Signed-off-by: Jiahao XU <[email protected]>

Test bytes in stdout first in test_read_large_file_bug

f69045b

Signed-off-by: Jiahao XU <[email protected]>

Add new dep libc v0.2.137

db04a66

Signed-off-by: Jiahao XU <[email protected]>

Fix Fd::as_raw_fd_or_null_fd: set pased fd to be blocking

5651daf

Signed-off-by: Jiahao XU <[email protected]>

Fix Fd::as_raw_fd_or_null_fd: Avoid breaking invariant of Pipe*

a0e7bce

Signed-off-by: Jiahao XU <[email protected]>

Add integration test test_read_large_file_bug2

5006c2a

Signed-off-by: Jiahao XU <[email protected]>

Rm test_read_large_file_bug2

a30e59d

Signed-off-by: Jiahao XU <[email protected]>

Fix RemoteChild::wait_with_output: Drop self after stdout/stderr is…

a74a534

… fully read Signed-off-by: Jiahao XU <[email protected]>

NobodyXu requested a review from jonhoo October 29, 2022 11:47

Improve doc of RemoteChild

e358ce1

Signed-off-by: Jiahao XU <[email protected]>

Rm unused imports in tests/openssh.rs

379750a

Signed-off-by: Jiahao XU <[email protected]>

jonhoo requested changes Oct 30, 2022

View reviewed changes

src/child.rs Outdated Show resolved Hide resolved

src/child.rs Outdated Show resolved Hide resolved

src/native_mux_impl/stdio.rs Outdated Show resolved Hide resolved

src/native_mux_impl/stdio.rs Show resolved Hide resolved

src/native_mux_impl/stdio.rs Outdated Show resolved Hide resolved

NobodyXu and others added 3 commits October 30, 2022 11:58

Update src/child.rs

e9ed758

Co-authored-by: Jon Gjengset <[email protected]>

Update src/child.rs

28ad64d

Co-authored-by: Jon Gjengset <[email protected]>

Fix Fd::new_owned: Make it unsafe & impl From<Pipe*> for Fd

6fd7cc0

Signed-off-by: Jiahao XU <[email protected]>

NobodyXu requested a review from jonhoo October 30, 2022 01:16

jonhoo reviewed Oct 30, 2022

View reviewed changes

src/native_mux_impl/stdio.rs Outdated Show resolved Hide resolved

NobodyXu added 2 commits October 31, 2022 10:31

Add saferty doc to Fd::new_owned

771d6d8

Signed-off-by: Jiahao XU <[email protected]>

Update comment in RemoteChild::wait_with_output

0bb7ad3

Signed-off-by: Jiahao XU <[email protected]>

NobodyXu requested a review from jonhoo October 30, 2022 23:38

jonhoo reviewed Oct 31, 2022

View reviewed changes

src/child.rs Outdated Show resolved Hide resolved

Update src/child.rs

527843e

NobodyXu requested a review from jonhoo October 31, 2022 02:03

Refactor: Invoke set_blocking in Command::spawn

139e070

Signed-off-by: Jiahao XU <[email protected]>

jonhoo requested changes Oct 31, 2022

View reviewed changes

src/native_mux_impl/command.rs Outdated Show resolved Hide resolved

NobodyXu added 2 commits October 31, 2022 16:23

Fix Command::spawn: Skip set_blocking for inherited

6d1d1ff

Signed-off-by: Jiahao XU <[email protected]>

Add comment in Command::spawn

c689253

Signed-off-by: Jiahao XU <[email protected]>

NobodyXu requested a review from jonhoo October 31, 2022 05:25

Only set_blocking only on OwnedFd

9a7b1f2

Do not `set_blocking` on `StdioImpl::inherit` as we do not own them and also do not `set_blocking` on `Stdio` constructed using `FromRawFd::from_raw_fd` Signed-off-by: Jiahao XU <[email protected]>

jonhoo requested changes Oct 31, 2022

View reviewed changes

src/stdio.rs Show resolved Hide resolved

src/native_mux_impl/stdio.rs Outdated Show resolved Hide resolved

Refactor stdio

155852f

Signed-off-by: Jiahao XU <[email protected]>

NobodyXu requested a review from jonhoo October 31, 2022 23:55

jonhoo reviewed Nov 7, 2022

View reviewed changes

src/native_mux_impl/stdio.rs Show resolved Hide resolved

src/stdio.rs Show resolved Hide resolved

NobodyXu added 2 commits November 7, 2022 11:39

Impl new fn Stdio::from_raw_fd_owned

6b1bcb3

Signed-off-by: Jiahao XU <[email protected]>

Mark FromRawFd impl for Stdio as deprecated

908141c

Signed-off-by: Jiahao XU <[email protected]>

NobodyXu requested a review from jonhoo November 7, 2022 00:46

Mark From<tokio_pipe::Pipe{Read, Write} for Stdio as deprecated

4d76786

Signed-off-by: Jiahao XU <[email protected]>

jonhoo approved these changes Nov 7, 2022

View reviewed changes

NobodyXu added 2 commits November 7, 2022 11:53

Improve doc

102573a

Signed-off-by: Jiahao XU <[email protected]>

Update changelog

4c8e61a

Signed-off-by: Jiahao XU <[email protected]>

jonhoo approved these changes Nov 7, 2022

View reviewed changes

jonhoo merged commit e1efac5 into master Nov 7, 2022

jonhoo deleted the fix/cut-off branch November 7, 2022 01:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed stdout/stderr cut off bug when using `RemoteChild::wait_with_output` #104

Fixed stdout/stderr cut off bug when using `RemoteChild::wait_with_output` #104

NobodyXu commented Oct 29, 2022 •

edited

Loading

jonhoo commented Oct 29, 2022

NobodyXu commented Oct 29, 2022

codecov-commenter commented Oct 29, 2022 •

edited

Loading

NobodyXu commented Oct 29, 2022

Fuuzetsu commented Oct 31, 2022

NobodyXu commented Oct 31, 2022

jonhoo commented Nov 7, 2022

NobodyXu commented Nov 7, 2022

NobodyXu commented Nov 7, 2022

jonhoo commented Nov 7, 2022

jonhoo commented Nov 7, 2022

jonhoo commented Nov 7, 2022

NobodyXu commented Nov 7, 2022

Fixed stdout/stderr cut off bug when using RemoteChild::wait_with_output #104

Fixed stdout/stderr cut off bug when using RemoteChild::wait_with_output #104

Conversation

NobodyXu commented Oct 29, 2022 • edited Loading

jonhoo commented Oct 29, 2022

NobodyXu commented Oct 29, 2022

codecov-commenter commented Oct 29, 2022 • edited Loading

Codecov Report

NobodyXu commented Oct 29, 2022

Fuuzetsu commented Oct 31, 2022

NobodyXu commented Oct 31, 2022

jonhoo commented Nov 7, 2022

NobodyXu commented Nov 7, 2022

NobodyXu commented Nov 7, 2022

jonhoo commented Nov 7, 2022

jonhoo commented Nov 7, 2022

jonhoo commented Nov 7, 2022

NobodyXu commented Nov 7, 2022

Fixed stdout/stderr cut off bug when using `RemoteChild::wait_with_output` #104

Fixed stdout/stderr cut off bug when using `RemoteChild::wait_with_output` #104

NobodyXu commented Oct 29, 2022 •

edited

Loading

codecov-commenter commented Oct 29, 2022 •

edited

Loading