Skip to content

Make rekillable consistent with unkillable #8581

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

Closed
wants to merge 6 commits into from

Conversation

flaper87
Copy link
Contributor

As for now, rekillable is an unsafe function, instead, it should behave
just like unkillable by encapsulating unsafe code within an unsafe
block.

This patch does that and removes unsafe blocks that were encapsulating
rekillable calls throughout rust's libs.

Fixes #8232

@flaper87
Copy link
Contributor Author

@bblum here it is 😃

@flaper87
Copy link
Contributor Author

@bblum I added a small test but I'm not 100% that's what is needed. I'll add more if so. I'd appreciate if you could elaborate more on the various cases you think worth testing. (I'm pretty new to rust).

Thanks a lot!

@bblum
Copy link
Contributor

bblum commented Aug 18, 2013

The test you added looks good but there should also be a test for the advertised behaviour, namely, that a task can get killed inside a rekillable block. Something like this:

let result = do task::try {
    do unkillable { do rekillable {
        let (port,chan) = comm::stream();
        do task::spawn { chan.send(()); fail!(); }
        port.recv(); // wait for child to exist
        port.recv(); // block forever, expect to get killed.
    } }
};
assert!(result.is_err());

You also have not added any code to the runtime that will actually make the task fail, which you will need to do if you want the tests to pass.

@huonw
Copy link
Member

huonw commented Aug 24, 2013

This needs a rebase (it no longer merges cleanly).

@flaper87
Copy link
Contributor Author

Will do!

I'm out and I'll be back tomorrow!!!

As for now, rekillable is an unsafe function, instead, it should behave
just like unkillable by encapsulating unsafe code within an unsafe
block.

This patch does that and removes unsafe blocks that were encapsulating
rekillable calls throughout rust's libs.

Fixes rust-lang#8232
@flaper87
Copy link
Contributor Author

rebased

bors added a commit that referenced this pull request Aug 27, 2013
As for now, rekillable is an unsafe function, instead, it should behave
just like unkillable by encapsulating unsafe code within an unsafe
block.

This patch does that and removes unsafe blocks that were encapsulating
rekillable calls throughout rust's libs.

Fixes #8232
@bors bors closed this Aug 27, 2013
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 this pull request may close these issues.

task::rekillable should not be unsafe, and just fail the task, not abort the runtime
4 participants