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

Vec dtor might want to try harder on failure #16135

Closed
brson opened this issue Jul 31, 2014 · 6 comments
Closed

Vec dtor might want to try harder on failure #16135

brson opened this issue Jul 31, 2014 · 6 comments
Labels
A-destructors Area: Destructors (`Drop`, …) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@brson
Copy link
Contributor

brson commented Jul 31, 2014

Although dtors should not fail (by convention), it can happen. The Vec dtor doesn't do anything to defend against one of its elements' failing.

Since there's no winning when dtors fail, it's not obvious that Vec should try to do anything, but for comparison, the I/O process and stream types both take defensive measures to try to make the best of similar situations.

This program leaks 4 boxes, the buffer, and fails to run 3 dtors:

struct F(Box<()>);

impl Drop for F {
    fn drop(&mut self) {
        println!("drop");
        fail!()
    }
}

fn main() {
    let _v = vec!(F(box () ()), F(box () ()), F(box () ()), F(box () ()));
}
@alexcrichton
Copy link
Member

This is... disturbing. We also discovered today that Rc and Arc leak their allocations on failure of the contained object's destructor.

In general it sounds like our "clean up on destructor failure" story isn't so hot when considering issues like #14875 as well.

@thestinger
Copy link
Contributor

I don't think it makes sense to take a further performance and complexity hit to improve (but not fix) support for something that's already broken.

@steveklabnik
Copy link
Member

Triage: this sitll only prints drop once, with some trivial updates to today's syntax.

@steveklabnik
Copy link
Member

Triage:

#![feature(box_syntax)]
struct F(Box<()>);

impl Drop for F {
    fn drop(&mut self) {
        println!("drop");
        panic!()
    }
}

fn main() {
    let _v = vec!(F(box ()), F(box ()), F(box ()), F(box ()));
}
steve@warmachine:~/tmp$ ./main 
drop
thread '<main>' panicked at 'explicit panic', main.rs:7
note: Run with `RUST_BACKTRACE=1` for a backtrace.

@bluss
Copy link
Member

bluss commented Mar 4, 2016

Since we did the Vec / RawVec split, it should now be the case that the RawVec allocation itself is always free'd even if one of the elements panic in Vec's drop.

Confirmed that it does deallocate the RawVec allocation using rustc 1.9.0-nightly (7b0b80a 2016-03-02).

@steveklabnik steveklabnik added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed A-libs labels Mar 24, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 21, 2017
@Mark-Simulacrum
Copy link
Member

I believe we handle this correctly today, in that we attempt to deallocate all elements regardless of failure:

struct F(u32);

impl Drop for F {
    fn drop(&mut self) {
        eprintln!("drop");
        if self.0 == 0 { panic!() }
    }
}

fn main() {
    let _v = vec!(F(1), F(0), F(1), F(1));
}
   Compiling playground v0.0.1 (file:///playground)
    Finished dev [unoptimized + debuginfo] target(s) in 0.66 secs
     Running `target/debug/playground`
drop
drop
thread 'main' panicked at 'explicit panic', src/main.rs:7:25
note: Run with `RUST_BACKTRACE=1` for a backtrace.
drop
drop

bors added a commit to rust-lang-ci/rust that referenced this issue Mar 31, 2024
… r=Veykril

Refactor extension to support arbitrary shell command runnables

Currently, the extension assumes that all runnables invoke cargo. Arguments are sometimes full CLI arguments, and sometimes arguments passed to a cargo subcommand.

Refactor the extension so that tasks are just a `program` and a list of strings `args`, and rename `CargoTask` to `RustTask` to make it generic.

(This was factored out of rust-lang#16135 and tidied.)
lnicola pushed a commit to lnicola/rust that referenced this issue Jun 23, 2024
Allow rust-project.json to include arbitrary shell commands for runnables

This is a follow-up on rust-lang#16135, resolving the feedback raised :)

Allow rust-project.json to include shell runnables, of the form:

```
{
  "build_info": {
    "label": "//project/foo:my-crate",
    "target_kind": "bin",
    "shell_runnables": [
      {
        "kind": "run",
        "program": "buck2",
        "args": ["run", "//project/foo:my-crate"]
      },
      {
        "kind": "test_one",
        "program": "test_runner",
        "args": ["--name=$$TEST_NAME$$"]
      }
    ]
  }
}

```

If these runnable configs are present for the current crate in rust-project.json, offer them as runnables in VS Code.

This PR required some boring changes to APIs that previously only handled cargo situations. I've split out these changes as commits labelled 'refactor', so it's easy to see the interesting changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-destructors Area: Destructors (`Drop`, …) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants