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

Failed asserting on Select drop #12042

Closed
ghost opened this issue Feb 5, 2014 · 7 comments
Closed

Failed asserting on Select drop #12042

ghost opened this issue Feb 5, 2014 · 7 comments
Labels
P-low Low priority

Comments

@ghost
Copy link

ghost commented Feb 5, 2014

This is reproduce able 100% with the following code. Looking at the disassembly the problem is that one of the handles drop function is called after the select drop is called.

use std::comm::{Port, Chan, Select};

fn wait_commands(cmd: &mut Port<Port<()>>, ports: &mut ~[Port<()>]) -> Port<()>
{
    let mut handles = ~[];
    let select = Select::new();
    let mut cmd_handle = select.add(cmd);
    for port in ports.mut_iter() {
        handles.push(select.add(port));
    }
    loop {
        let id = select.wait();
        if id == cmd_handle.id {
            let port = cmd_handle.recv();
            println!("{:?}", port);
            return port;
        }
        for h in handles.mut_iter() {
            if h.id == id {
                break;
            }
        }
    }
}

fn thread(cmd: Port<Port<()>>)
{
    let mut cmd = cmd;
    let mut ports: ~[Port<()>] = ~[];
    loop {
        let port = wait_commands(&mut cmd, &mut ports);
        println!("{:?}", port);
        ports.push(port);
    }
}

fn main()
{
    let (p, c) = Chan::new();
    spawn(proc() {
        thread(p);
    });
    let (new_p0, new_c0): (Port<()>, Chan<()>) = Chan::new();
    let (new_p1, new_c1): (Port<()>, Chan<()>) = Chan::new();
    c.send(new_p0);
    c.send(new_p1);
    new_c0.send(());
    new_c1.send(());
}

output

std::comm::Port<()>{queue: SPSC(std::sync::spsc_queue::Consumer<(),std::comm::Packet>{state: std::sync::arc::UnsafeArc<std::sync::spsc_queue::State<(),std::comm::Packet>>{data: (0x7f435c001640 as *mut ())}}), marker: std::kinds::marker::NoFreeze}
std::comm::Port<()>{queue: SPSC(std::sync::spsc_queue::Consumer<(),std::comm::Packet>{state: std::sync::arc::UnsafeArc<std::sync::spsc_queue::State<(),std::comm::Packet>>{data: (0x7f435c001640 as *mut ())}}), marker: std::kinds::marker::NoFreeze}
std::comm::Port<()>{queue: SPSC(std::sync::spsc_queue::Consumer<(),std::comm::Packet>{state: std::sync::arc::UnsafeArc<std::sync::spsc_queue::State<(),std::comm::Packet>>{data: (0x7f435c001740 as *mut ())}}), marker: std::kinds::marker::NoFreeze}
task '<unnamed>' failed at 'assertion failed: self.head.is_null()', /build/buildd/rust-nightly-201402020405~3e39e3e~precise/src/libstd/comm/select.rs:298
@alexcrichton
Copy link
Member

My previous thoughts about destructors are not providing the soundness that I thought I was getting. I would expect code like this to not compile:

struct Foo {
    a: int
}

struct Bar<'a> {
    a: &'a Foo,
}

impl Drop for Foo {
    fn drop(&mut self) {
        println!("dropping foo");
    }
}

#[unsafe_destructor]
impl<'a> Drop for Bar<'a> {
    fn drop(&mut self) {
        println!("dropping bar {}", self.a.a);
    }
}

fn main() {
    let mut a = ~[];
    let b = Foo { a: 2 };
    a.push(Bar { a: &b });
}

The output is

dropping foo
dropping bar 2

Which is clearly a use-after-free. I presume that this is why I had to write #[unsafe_destructor]. If you re-order

    let mut handles = ~[];
    let select = Select::new();

to

    let select = Select::new();
    let mut handles = ~[];

your code should work alright.

cc @nikomatsakis, just want to make sure that this isn't an unknown bug in destructors right now. Also, could any of the new marker types help me with this bug?

@alexcrichton
Copy link
Member

Nominating.

@nikomatsakis
Copy link
Contributor

This is why you had to write unsafe destructor. This is not an unknown bug -- you should never write unsafe destructor on a type that "escapes" to the wild. It is only valid if you control exactly where that type is instantiated. #8861 would possibly help here.

@pnkfelix
Copy link
Member

pnkfelix commented Feb 6, 2014

Not a 1.0 blocker, P-low. #8861 subsumes the high priority dtor semanitcs issue implicit here.

@pnkfelix pnkfelix added P-low and removed I-nominated labels Feb 6, 2014
@reem
Copy link
Contributor

reem commented Aug 8, 2014

If this is superseded by #8861, can it be closed? It seems like this is mostly a mistake and not really a bug.

@pnkfelix
Copy link
Member

man this is some old code in this example I need to forward port here!

@pnkfelix
Copy link
Member

Okay, here is my attempt to forward port the original code: http://is.gd/kk3Fb9

It now produces an error that select does not live long enough.

And if you do the suggested rewrite from #12042 (comment) illustrated here: http://is.gd/aSNKtF then it compiles (and times out in the playpen)! Score one for #8861! It only took us a year to get there!

So, i am closing.

bors added a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2022
…-from-lang-config, r=jonas-schievink

fix: remove angle brackets from language configuration

This should fix rust-lang/rust-analyzer#12034

It looks like we shouldn't add any characters here that can be ambiguous, because it can make the editor highlight unrelated characters. This needs a parser to be correct, so the language server is the right place, not the editor. Upstream LSP feature request: microsoft/language-server-protocol#672 (but it might be possible to implement this as an extension today, as long as that doesn't conflict with the built-in highlighting).
flip1995 pushed a commit to flip1995/rust that referenced this issue Jan 11, 2024
…int-listing, r=Manishearth

fix: metadata-collector lists wrong affected lints

fixes rust-lang#12042

This PR addresses the issue where the `metadata collector` incorrectly generates the `Affected Lints` section when a comma is included in the configuration documentation.

I made adjustments; however, if the `/// Lint: SOMETHING` section ends with `.` it always produces the correct output.
For example,
```rust
/// Lint: PUB_UNDERSCORE_FIELDS
```
should be
```rust
/// Lint: PUB_UNDERSCORE_FIELDS.
```

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-low Low priority
Projects
None yet
Development

No branches or pull requests

4 participants