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

Transferred FDs aren't properly closed #235

Closed
YaLTeR opened this issue Jan 25, 2019 · 1 comment · Fixed by #236
Closed

Transferred FDs aren't properly closed #235

YaLTeR opened this issue Jan 25, 2019 · 1 comment · Fixed by #236

Comments

@YaLTeR
Copy link
Contributor

YaLTeR commented Jan 25, 2019

Consider receiving data over a zwlr_data_control_offer:

// Create a pipe for content transfer.
let (mut read, write) = pipe().expect("Error creating a pipe");

// Start the transfer.
offer.receive(mime_type, write.as_raw_fd());
queue.sync_roundtrip().expect("Error doing a roundtrip");
drop(write);

// Read the contents.
let mut contents = vec![];
read.read_to_end(&mut contents)
    .expect("Error reading clipboard contents");

stdout().write_all(&contents)
        .expect("Error writing contents to stdout");

Using native_lib this works, using the Rust implementation this blocks on read. I highly suspect this is due to wayland-commons duping FDs before writing and subsequently never closing them over here:

// we store all fds we dup-ed in this, which will auto-close
// them on drop, if any of the `?` early-returns
let mut pending_fds = FdStore::new();
// write the contents in the buffer
for arg in &self.args {
// Just to make the borrow checker happy
let old_payload = payload;
match *arg {
Argument::Int(i) => payload = write_buf(i as u32, old_payload)?,
Argument::Uint(u) => payload = write_buf(u, old_payload)?,
Argument::Fixed(f) => payload = write_buf(f as u32, old_payload)?,
Argument::Str(ref s) => {
payload = write_array_to_payload(s.as_bytes_with_nul(), old_payload)?;
}
Argument::Object(o) => payload = write_buf(o, old_payload)?,
Argument::NewId(n) => payload = write_buf(n, old_payload)?,
Argument::Array(ref a) => {
payload = write_array_to_payload(&a, old_payload)?;
}
Argument::Fd(fd) => {
let old_fds = fds;
let dup_fd = dup_fd_cloexec(fd).map_err(MessageWriteError::DupFdFailed)?;
pending_fds.push(dup_fd);
fds = write_buf(dup_fd, old_fds)?;
payload = old_payload;
}
}
}
// we reached here, all writing was successful
// no FD needs to be closed
pending_fds.clear();

Changing the last line to drop(pending_fds) isn't valid either because we can't close the FDs before the server received them.

@YaLTeR
Copy link
Contributor Author

YaLTeR commented Feb 16, 2021

So, looks like this came back at one point because I get the exact same issue on 0.28.3.

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 a pull request may close this issue.

2 participants