Comments (12)
@jeremyjh @Drakulix @polyfractal , would you like to add your opinion?
from mioco.pre-0.9.
I think by default it should let panic unwind through the scheduler's thread, but there should be a mechanism to catch panic before it crashes the whole scheduler - just as you can in native threads.
from mioco.pre-0.9.
That this library provides panic handling at all is not really idiomatic Rust. A panic implies a critical point of failure, which cannot be recovered in a meaningful way, most of the time related to unsafe code or knowingly invalidating a simple invariant.
That recover and propagate is in place is mostly only related to the fact, that it causes undefined behavior (most of the time causing a crash) to unwind into or through foreign (non-rust) code.
However I do understand, that in server and production related code, (which is what mioco is somewhat aiming for with evented IO,) you need to protect your whole program against crashing. Even if a panic should never happen.
BUT there is nothing stopping the library user to insert a recover statement in their coroutine themselves. Especially since this does not seem to be very costly at the moment.
I would say toss all of that panic related logic out of the code (except for the necessary propagate into the scheduler thread) and let the user handle it. This would clean up a good amount of code and gives us the opportunity to do something more useful with the exit_notificator
. Maybe it is even possible to send the actual return value of the coroutine through it, which I would find a lot more useful.
I would even say the current API does encourage you to throw a panic in case of an error, because it is easier to get that information through the exit_notificator
, then creating and passing a newly created mailbox
into the coroutine just to send a completion or error message.
And this is not Java, we do not want panics all over the place.
Anyway I am also in favor of changing the default behavior.
from mioco.pre-0.9.
Panic has always been intended to be handled at the Task level in Rust; I think that is more idiomatic than using recover statements in the thread that may raise a panic.
from mioco.pre-0.9.
I am not sure, if I understand your last comment @jeremyjh . It seems to be contradictory to your last one.
Handling at task level would mean, let the user wrap their tasks, if they may panic. Handling at a thread level would be closer to the current behavior. So whats your preference?
from mioco.pre-0.9.
Sorry, I'm using old terminology - I should just say thread. We used to have task::try in the standard library, which was the only way to catch a panic - it spawned a new thread and would catch a panic in that thread if it happened - there was no way to recover in the same thread.
I think this was removed because it encouraged people to use heavy-weight OS threads just to be able to catch a panic. In mioco, the older model makes more sense to me because cos are lightweight. So all I am asking for is an equivalent of the old task::try, that would spawn a new coroutine and let it contain a panic result and handle it how they wished.
See http://smallcultfollowing.com/rust-int-variations/imem-umem/guide-tasks.html for the old behavior.
from mioco.pre-0.9.
Alright that makes sense.
You have a point, it is clearly easier to handle the panic through a try
-like interface, then through a very verbose recover
statement.
Also my argument seems to be invalid, I forgot, that coroutines already return a Result
.
@dpc Do you think it might be possible to make ExitStatus generic over the Results Ok-Variant? Maybe even over the Err Variant.
e.g.
pub enum ExitStatus<R: Sized + Send>
{
Exit(Arc<Result<R>>,
Panic,
Killed,
}
from mioco.pre-0.9.
@jeremyjh : You made a good point. Thanks.
I think there's technically no drawback from keeping panic handling, and it seems it has it's own advantages.
@Drakulix : It would be possible, but it seems it would require whole mioco instance to be parametrized over R
, so that all coroutines return same type. Which is maybe OK... . Maybe it wouldn't even affect a lot of types. And even if it does, it shouldn't hurt.
Or maybe some Into
/ From
/ Any
into the mix would allow more flexible approach, where any coroutine could return anything (at least type wise), and if it's not something expected ExitStatus::Unkown
result would be returned...
from mioco.pre-0.9.
I think we should just stick to the current implementation then. Result
already is the right type to encode Success and Failure, I do not think parametric mioco instances add much more flexibility to the overall API, but make the code base much more verbose and complex.
We should just decide, if we catch panics by default or not.
from mioco.pre-0.9.
My vote would be not catch them by default.
On Thursday, February 25, 2016, Victor Brekenfeld [email protected]
wrote:
I think we should just stick to the current implementation then. Result
already is the right type to encode Success and Failure, I do not think
parametric mioco instances add much more flexibility to the overall API,
but make the code base much more verbose and complex.We should just decide, if we catch panics by default or not.
—
Reply to this email directly or view it on GitHub
https://github.com/dpc/mioco/issues/90#issuecomment-188859086.
from mioco.pre-0.9.
+1
from mioco.pre-0.9.
All right then. Default is to catch them, there's an option to disable that. I guess we can close.
from mioco.pre-0.9.
Related Issues (20)
- Optimising repeated select!s HOT 1
- Is `mioco::get_userdata` sound? HOT 17
- Error handling for TcpStream::connect to non-listening port HOT 3
- `Receiver` should not be always `Send` HOT 5
- Channel panics when mixing `recv()` and `select!` HOT 5
- `UnixListener` is not usable. HOT 2
- Implement `sync_channel`
- `select!` takes at least one tick, which is super slow.
- No way to cancel notifications - select! on a lot of channels in loop can accumulate tons of notifications. HOT 1
- Change the documentation to remove the need for Rust Nightly HOT 2
- Missing openwrt/mipsel support HOT 3
- Prevent using mioco types from multiple thread(coroutines) at the same time. HOT 3
- clear documentation on handling panics HOT 17
- Travis panic
- Test fail in 0.8.0 with rustc stable and beta HOT 1
- Mioco is blocked polling for IO while there are runnable tasks HOT 1
- Mioco compilation with channels and streams fails. HOT 6
- reader.try_read() blocks forever after reading a few bytes
- Colerr mioco-related crash on short program.
- program crashed when run long time HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from mioco.pre-0.9.