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

FdSet is difficulty to use #2130

Closed
ensc opened this issue Sep 16, 2023 · 1 comment · Fixed by #2136
Closed

FdSet is difficulty to use #2130

ensc opened this issue Sep 16, 2023 · 1 comment · Fixed by #2136

Comments

@ensc
Copy link

ensc commented Sep 16, 2023

Code like

use nix::sys::{ select, signalfd::{self, SigSet} };
fn main() {
    let mut fd_sig = signalfd::SignalFd::new(&SigSet::empty()).unwrap();
    let mut fd_tmp = signalfd::SignalFd::new(&SigSet::empty()).unwrap();

    loop {
	let mut fdset = select::FdSet::new();

	fdset.insert(&fd_tmp);
	fdset.insert(&fd_sig);

	select::select(None, &mut fdset, None, None, None).unwrap();

	if fdset.contains(&fd_sig) {
	    fd_sig.read_signal().unwrap();
	}

	if fdset.contains(&fd_tmp) {
	}
    }
}

fails to compile with

error[E0502]: cannot borrow `fd_sig` as mutable because it is also borrowed as immutable
  --> src/main.rs:17:6
   |
12 |     fdset.insert(&fd_sig);
   |                  ------- immutable borrow occurs here
...
17 |         fd_sig.read_signal().unwrap();
   |         ^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
...
20 |     if fdset.contains(&fd_tmp) {
   |        ----- immutable borrow later used here

Workaround is to store results of all used fdset.contains() in temporary variables like

        ...
	select::select(None, &mut fdset, None, None, None).unwrap();

        let ev_sig = fdset.contains(&fd_sig);
        let ev_tmp = fdset.contains(&fd_tmp);

But this is ugly.

Would it be possible to change the FdSet functions to

-    pub fn insert<Fd: AsFd>(&mut self, fd: &'fd Fd) {
+    pub fn insert<Fd: AsFd + 'fd>(&mut self, fd: Fd) {

?

@asomers
Copy link
Member

asomers commented Sep 22, 2023

Yes, your analysis is correct. Would you care to submit a PR?

github-merge-queue bot pushed a commit that referenced this issue Oct 1, 2023
)

* Relax lifetime requirements for FdSet::{insert, remove, contains}

Fixes #2130

* Take BorrowedFd as the argument for FdSet::{insert, remove, contains}

&AsFd doesn't work because there are 'static types, like std::fs::File,
which implement AsFd.

* fix changelog & remove unused entries

* fix wrong PR number

---------

Co-authored-by: Steve Lau <stevelauc@outlook.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 a pull request may close this issue.

2 participants