[context review]
First of all, I think the library as some issues and it should not be accepted as it is, but see below. - What is your evaluation of the design? The interface is pretty simple and it seems fine on a first evaluation, but I think there are some issues. The first issue is that the destructor of the context class will (at least by default) happily destroy the stack. This unfortunately will leak any resource owned by any object on the stack; This is not unlike destroying an allocator before having destroyed all allocated object. A stack can only be safely deallocated if either has been completely unwound or if it doesn't own any resources (note that is not enough to just destroy any stack allocated object). Context should either: a) provide an interface to unwind the stack (by automatically trowing a special exception for example), or b) detect whether the stack has not been unwound and assert in debug mode in the destructor. As it is, is not clear the level of abstraction this class wants to be. It looks like is trying, using RAII, to manage context creation and destruction safely, giving a false sense of security. Context is neither copyable nor movable; I can understand the reason for making it non-copyable, but I see no reason why it shouldn't be movable. Without either capability it is hard to make context interoperate with existing code without forcing to dynamic allocation. Context is templated on both the context implementation and the stack. The first problem is that the concept requirements for the Stack template arguments are not stated. Second, the stack type should not be part of the context type: the primary reason for specifying different stacks is for option of having the guard page: enabling this should not change the type of Context and it should possible to specify it at runtime. Basically I'm advocating for type erasure; note that it can (and should) be implemented without additional memory allocation. The trampoline function can be defined in such a way that the custom deleter will be invoked after the stack is unwound. Regarding the CtxT type, instead of this template parameter there should be a program wide default controllable at compile time by a macro; I saw elsethread that Oliver is already leaning on this idea. As it is the context class is IMHO flawed and I'm against its inclusion in boost right. I'll be very happy to change my vote if I see a better implementation. A good abstraction layer around the low level context, among other things, should support clean unwind of the stack (or detecting an not-unwound stack), generic functors (and not just function pointers) for type safety and custom deleters. On the other hand the detail fcontext class seems a good, if maybe not complete, portable implementation of a low level ucontext-like API. I would be in favor of a simpler version of context which consists solely on the fcontext class (with a guarantee that this type is a POD), plus the make_context and swap_context functions. A user will have lower expectations from such a low level API and at the same time will give more freedom for other people to implement on top of it. A safe context class would just be one of the client of the API, other library authors could either use Context of the underlying fcontex directly depending on wether they need the speed or safety. I think this basic API could still be tweaked [but my vote won't be conditional on that] to add extra functionality: * Instead of supporting a 'fc_link' a-la ucontext, have the callback function return a new context (it is void now). 'fc_link' is useful only on some limited cases (usually when you have an external scheduler managing your contextes), while the having the callback return the 'next continuation' has more applications (note that fc_link can be implemented easily in term of the continuation returning variant, but the reverse is harder). This is very useful, for example, to implement some kind of 'exit_to' functionality (unwind the stack and restore another context), a key piece for safe context destruction. * Give to fcontext_swap the ability to exchange a parameter with the other context: fcontext_parm_t swap_fcontext( fcontext_t * ofc, fcontext_t const* nfc, fcontext_parm_t parm) where fcontext_parm_t is either union fcontext_parm_t { void * ptr; int int_; }; or simply intptr_t; Swap_fcontext saves the current context in ocf and restores the context ncf as usual; additionally, if ofc was 'captured' on a call to swap_fcontext, this call will return the value of parm on the original context. This extra parameter is equivalent to the second parameter of longjmp. If ofc was created with make_fcontext, 'parm' will be an extra parameter to the function. Note that because you can't mix and match parameterful and parameterless swap_fcontext, you should only have the former as it can be potentially implemented with little or no overhead. * Add a variant of swap_context that allows executing a function on the destination context (in this case 'parm' would be passed parameter to this function. This is mostly useful for injecting an exception on the destination context. This can be potentially implemented on top of the existing functionality, but would slow down every context switch; a possible efficient implementation would manipulate the destination stack to add a call to the function on top of it, so that the cost is paid only when required. All the previous improvements are implementable unobtrusively on top of the exiting API, but such implementations will be much less efficient. I'm suggesting that they should be part of the basic API becausecan be implemented with zero or near zero overhead when not used. - What is your evaluation of the implementation? The implementation seems reasonably efficient, but I have seen elsethread suggestions to make the constructor more expensive (read multiple memory allocations) in exchange for flexibility; this is IMHO wrong, the constructor should not be assumed to be a non critical operation. In fact I think that most of the proposals can be efficiently implemented without additional memory allocation by reusing the already available stack (which can be reused). Most of the 'throw' statements in the context class should actually be asserts. Make_fcontext requires a call to get_fcontex first, a-la ucontext: there is no reason for this, just merge get_context with make_context (and drop the unused set_fcontext). The posix ucontext wapper and the windows fiber wrapper should be made API compatible with fcontext. Regarding the assembler implementations of fcontext, I think they could be simplified if the fcontext itself was represented as a single pointer to the top of the stack. This would save some instructions to move the source address from the stack return address to the fcontext struct and back. The i386 implementation could use register calling convention to simplify the implementation and speed it up. Both the i386 and amd64 asm implementation use the 'ret' instruction to restore instruction pointer of the destination context. This will always be mispredicted (costing 20 cycles or more, easily more than the rest of the swapping code). Just jmping to the the destination will save a push and will give the CPU a chance to predict the jmp (modern CPUs have some dynamic prediction capability for indirect jumps). Bot the i386 and amd64 asm implementation save and restore the fpu control word (the amd64 both x87 and SSE2 control words). Last time I timed it, this was a relatively an expensive operation, not sure about current architectures; I do not think this should be saved and restored: the compiler will treat it as a caller clobbered variable and will take care of it in case of implicit manipulations. The user should be prohibited from changing it across a swap_fcontext call. As a minimum, there should be a compile time option to disable the saving. Note that many posix implementation of the ucontext API need to restore it (along with the full register set) because on traditional unices a context could be captured asynchronously by a signal, but it is not the case with boost.context. BTW, the current code is already inconsistent because it is saving the SSE2 control world on amd64 but not on i386. For posix systems lacking ucontext, fcontext can be implemented using the sigaltstack trick (this can actually be faster than ucontext if the non signal restoring _longjmp is used). For reference see: http://www.gnu.org/software/pth/rse-pmt.ps Finally, an implementation on top of boost threads would be nice. It will be very slow, but useful for portability and when performance is not an issue; also won't probably be much slower than ucontext. - What is your evaluation of the documentation? The documentation is very minimalist and should be expanded. It assumes the user is already familiar with the context swapping concept. The posix ucontext APIs man page are a good start. The fcontext class should be documented. The first time I read the documentation I understood that the constructor captured the current context a-la setjmp (a very bad idea, IMHO). I had to read the implementation to convince myself it was not the case. - What is your evaluation of the potential usefulness of the library? This is definitely not going to be one of the most used boost libraries, but it is a required piece to implement other libraries like coroutines, user lever threading, tasks, etc. - Did you try to use the library? With what compiler? Did you have any problems? I didn't try to use the library. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I did an in-depth study of the implementation. - Are you knowledgeable about the problem domain? I consider myself fairly experienced in this domain: As part of the GSoC project Boost.Coroutine I did some pretty in depth research on the topic and I implemented a very similar functionality; BTW, I see that Context borrows many ideas from Boost.Coroutine implementation (some of the tricks in the fiber wrapper look quite familiar); a small acknowledgement would be appreciated :). -- gpd
Hello,
The first issue is that the destructor of the context class will (at least by default) happily destroy the stack. This unfortunately will leak any resource owned by any object on the stack; This is not unlike destroying an allocator before having destroyed all allocated object.
This was already described in the docu.
A stack can only be safely deallocated if either has been completely unwound or if it doesn't own any resources (note that is not enough to just destroy any stack allocated object). Context should either: a) provide an interface to unwind the stack (by automatically trowing a special exception for example), or b) detect whether the stack has not been unwound and assert in debug mode in the destructor.
Because boost.context should be a thin wrapper/building block the higher abstractions are responsible. If the execution context was yielded in the middle of its exec path it would require to jump back throw an exception (whic hmay consume a lot of CPU cycles). Is this realy required? Couldn't it be reasonable for some implementations only to deallocate the stack?
As it is, is not clear the level of abstraction this class wants to be. It looks like is trying, using RAII, to manage context creation and destruction safely, giving a false sense of security.
see above
Context is neither copyable nor movable; I can understand the reason for making it non-copyable, but I see no reason why it shouldn't be movable. Without either capability it is hard to make context interoperate with existing code without forcing to dynamic allocation.
the reason not to support move-semantics relies on the support of ucontext. ucontext_t can not be copied (as required by move-semantics) because the struct ucontext_t is is implementation defined and may vary from release to release. Many man pages of several OS definetly warn for copying ucontext_t (http://nixdoc.net/man-pages/HP-UX/man2/makecontext.2.html).
Context is templated on both the context implementation and the stack. The first problem is that the concept requirements for the Stack template arguments are not stated.
I describe the requirements in the docu (implicit interface, move-semantics). I'm not sure which additional descriptions are required.
Second, the stack type should not be part of the context type: the primary reason for specifying different stacks is for option of having the guard page: enabling this should not change the type of Context and it should possible to specify it at runtime.
But enabling a guard page with mprotect() requires that the memory was allocated with mmap() othwerwise the behaviour is unspecified.
The trampoline function can be defined in such a way that the custom deleter will be invoked after the stack is unwound.
I'm not so happy with automatically stack unwinding. It should be done by higher abstractions (see my notes above).
Regarding the CtxT type, instead of this template parameter there should be a program wide default controllable at compile time by a macro; I saw elsethread that Oliver is already leaning on this idea.
yes - the second argument will be removed
A good abstraction layer around the low level context, among other things, should support clean unwind of the stack (or detecting an not-unwound stack), generic functors (and not just function pointers) for type safety and custom deleters.
I think this is discussible: I think that detecting an unwound stack would be a good addition. But Ive some concerns regarding to the other points: automatic stack unwinding, generic functors. Thsi should handled/implemented by this low-layer lib. For instance generic functors would require an additional allocation and some higher abstractions might not use/want such generic functors but would have to pay for it.
On the other hand the detail fcontext class seems a good, if maybe not complete, portable implementation of a low level ucontext-like API. I would be in favor of a simpler version of context which consists solely on the fcontext class (with a guarantee that this type is a POD), plus the make_context and swap_context functions. A user will have lower expectations from such a low level API and at the same time will give more freedom for other people to implement on top of it. A safe context class would just be one of the client of the API, other library authors could either use Context of the underlying fcontex directly depending on wether they need the speed or safety.
I was already requested to provide a C-lib regarding to fcontext.
I think this basic API could still be tweaked [but my vote won't be conditional on that] to add extra functionality:
* Instead of supporting a 'fc_link' a-la ucontext, have the callback function return a new context (it is void now). 'fc_link' is useful only on some limited cases (usually when you have an external scheduler managing your contextes)
this is what boost.tasklet does
Most of the 'throw' statements in the context class should actually be asserts.
was my first thought too - but what happend if the user compiles it with NDEBUG?
Make_fcontext requires a call to get_fcontex first, a-la ucontext: there is no reason for this, just merge get_context with make_context (and drop the unused set_fcontext).
this was required for the Windows fcontext implementation
The posix ucontext wapper and the windows fiber wrapper should be made API compatible with fcontext.
could you explain it please - I'm uncertain what do you mean.
Bot the i386 and amd64 asm implementation save and restore the fpu control word (the amd64 both x87 and SSE2 control words).
The SYSV AMD64 ABI requires that the SSE2 control and status word and the x87 control word have to preserved across function calls.
BTW, the current code is already inconsistent because it is saving the SSE2 control world on amd64 but not on i386.
I didn't read in the SYSV I386 ABI document about the fact that it requires to preserve SSE2 stuff.
For posix systems lacking ucontext, fcontext can be implemented using the sigaltstack trick (this can actually be faster than ucontext if the non signal restoring _longjmp is used). For reference see: http://www.gnu.org/software/pth/rse-pmt.ps
I'll take a look at it - but AFAIK setjmp/longjmp are not required to preserve the stackframe. Maybe it will not work on all platforms.
The fcontext class should be documented.
even if it is a implementation detail? regards, Oliver -- GMX DSL Doppel-Flat ab 19,99 Euro/mtl.! Jetzt mit gratis Handy-Flat! http://portal.gmx.net/de/go/dsl
On Fri, Mar 25, 2011 at 7:34 AM, Oliver Kowalke <oliver.kowalke@gmx.de> wrote:
A stack can only be safely deallocated if either has been completely unwound or if it doesn't own any resources (note that is not enough to just destroy any stack allocated object). Context should either: a) provide an interface to unwind the stack (by automatically trowing a special exception for example), or b) detect whether the stack has not been unwound and assert in debug mode in the destructor.
Because boost.context should be a thin wrapper/building block the higher abstractions are responsible. If the execution context was yielded in the middle of its exec path it would require to jump back throw an exception (whic hmay consume a lot of CPU cycles). Is this realy required? Couldn't it be reasonable for some implementations only to deallocate the stack?
It is very rare (but of course possible) that you want to deallocate the stack without unwinding it. As I said, I think that the current context class is at the wrong abstraction level: it should either not provide RAII at all (I.e. a plain C interface) or a complete safe implementation.
Context is neither copyable nor movable; I can understand the reason for making it non-copyable, but I see no reason why it shouldn't be movable. Without either capability it is hard to make context interoperate with existing code without forcing to dynamic allocation.
the reason not to support move-semantics relies on the support of ucontext. ucontext_t can not be copied (as required by move-semantics) because the struct ucontext_t is is implementation defined and may vary from release to release. Many man pages of several OS definetly warn for copying ucontext_t (http://nixdoc.net/man-pages/HP-UX/man2/makecontext.2.html).
Hum, I seem to remember reading of such a limitations, but a quick search didn't return anything. Also I couldn't find anything relevant on the Single Unix Specification. Anyways, you can still allocate the ucontext dynamically; it will be more expensive, but ucontext is already slow enough that it won't matter. Or you could allocate it on the provided stack. I see no reason to cripple the interface because of limitations of a specific implementation.
Context is templated on both the context implementation and the stack. The first problem is that the concept requirements for the Stack template arguments are not stated.
I describe the requirements in the docu (implicit interface, move-semantics). I'm not sure which additional descriptions are required.
Can you point me to these requirements? I see the definition of the protected_stack interface, but not of the concept requirements for StackT. Maybe I'm looking at an old version of the docs. I'm looking at this: http://ok73.ok.funpic.de/boost/libs/context/doc/html/context/context_managem...
Second, the stack type should not be part of the context type: the primary reason for specifying different stacks is for option of having the guard page: enabling this should not change the type of Context and it should possible to specify it at runtime.
But enabling a guard page with mprotect() requires that the memory was allocated with mmap() othwerwise the behaviour is unspecified.
How that prevents the stack being from being specifiable at runtime? What I'm asking for is for the class not to be templated on the stack type but only the constructor.
A good abstraction layer around the low level context, among other things, should support clean unwind of the stack (or detecting an not-unwound stack), generic functors (and not just function pointers) for type safety and custom deleters.
I think this is discussible: I think that detecting an unwound stack would be a good addition. But Ive some concerns regarding to the other points: automatic stack unwinding, generic functors. Thsi should handled/implemented by this low-layer lib. For instance generic functors would require an additional allocation and some higher abstractions might not use/want such generic functors but would have to pay for it.
Why generic functions would require an additional allocation? You can trivially build them on top of the existing interface. Again, it is fine for the C interface to only support pointers, but a RAII wrapper should be complete.
On the other hand the detail fcontext class seems a good, if maybe not complete, portable implementation of a low level ucontext-like API. I would be in favor of a simpler version of context which consists solely on the fcontext class (with a guarantee that this type is a POD), plus the make_context and swap_context functions. A user will have lower expectations from such a low level API and at the same time will give more freedom for other people to implement on top of it. A safe context class would just be one of the client of the API, other library authors could either use Context of the underlying fcontex directly depending on wether they need the speed or safety.
I was already requested to provide a C-lib regarding to fcontext.
Great. This should be the low level interface I'm talking about: leave all the stack handling to the user and only provide a context switching implementation. On the other hand, the context class (built on top of this low level interface), should provide a much higher degree of safety.
I think this basic API could still be tweaked [but my vote won't be conditional on that] to add extra functionality:
* Instead of supporting a 'fc_link' a-la ucontext, have the callback function return a new context (it is void now). 'fc_link' is useful only on some limited cases (usually when you have an external scheduler managing your contextes)
this is what boost.tasklet does
Are you saying that boost tasklet provides a scheduler or that it support for 'next continuation' returning functions?
Most of the 'throw' statements in the context class should actually be asserts.
was my first thought too - but what happend if the user compiles it with NDEBUG?
UB of course, as usual when removing assert and invariants/preconditions are broken. A quick grep finds 5 uses of throw, all in context.hpp: - context constructor from function and chaining constructor: if ( ! stack_) throw invalid_stack(); Just require stack to be valid as a precondition (and assert on debug builds). The user should be able to track the validity of its stacks anyway. if( ! base) throw bad_alloc(); it is not obvious at all that stack_.address() (used to initialize base) is allocating memory; In fact I see no reason it should, nor do the protected_stack_{posix,windows} implementations. Just require that stack_.address() returns a non null pointer. - jump_to Same as above: having a valid stack should be a precondition for calling this function. Note that here it is particularly important. This function is performance critical and should be as simple as possible.
Make_fcontext requires a call to get_fcontex first, a-la ucontext: there is no reason for this, just merge get_context with make_context (and drop the unused set_fcontext).
this was required for the Windows fcontext implementation
Can you elaborate on that? It is not obvious to me why the two functions cannot be merged together on this platform...
The posix ucontext wapper and the windows fiber wrapper should be made API compatible with fcontext.
could you explain it please - I'm uncertain what do you mean.
Ok, just forget about this. What I mean is that the library should provide a C api. Something like this: struct mycontext_t; // POD, memcpy-able. typedef mycontext_t fn_t(intptr_t); extern "C" intptr_t swap_mycontext( mycontext_t * ofc, mycontext_t const* nfc, intptr_t arg); // as swap_mycontext but with function injection. extern "C" intptr_t swap_mycontext2( mycontext_t * ofc, mycontext_t const* nfc, intptr_t arg, intptr_t (*chain)(intptr_t)); extern "C" intptr_t make_mycontext( mycontext_t * fc, fn_t * f, intptr_t p); // Destroy any other memory allocated by fc. Does not deallocate the stack. If stack is not unwound, the user better know // what he is doing extern "C" intptr_t destroy_mycontext( mycontext_t * fc); Protected_stack should be provided as is (plus a C interface if you really feel is useful), as it is good enough right now.
Bot the i386 and amd64 asm implementation save and restore the fpu control word (the amd64 both x87 and SSE2 control words).
The SYSV AMD64 ABI requires that the SSE2 control and status word and the x87 control word have to preserved across function calls.
You are right. For correctness you need to preserve them. It would be nice to have an option to remove these save/restore when the user does not manipulate the control word or it manipulates it program wise.
BTW, the current code is already inconsistent because it is saving the SSE2 control world on amd64 but not on i386.
I didn't read in the SYSV I386 ABI document about the fact that it requires to preserve SSE2 stuff.
I think this is still the official SYSV i386 abi document: http://www.sco.com/developers/devspecs/abi386-4.pdf but it doesn't mention SSE at all, so it can be complete. Do you have any other reference?
For posix systems lacking ucontext, fcontext can be implemented using the sigaltstack trick (this can actually be faster than ucontext if the non signal restoring _longjmp is used). For reference see: http://www.gnu.org/software/pth/rse-pmt.ps
I'll take a look at it - but AFAIK setjmp/longjmp are not required to preserve the stackframe. Maybe it will not work on all platforms.
It is a very dirty hack which is not guaranteed to work anywhere, but in practice is much more portable than ucontext_t. -- gpd
Giovanni Piero Deretta wrote:
Bot the i386 and amd64 asm implementation save and restore the fpu control word (the amd64 both x87 and SSE2 control words).
The SYSV AMD64 ABI requires that the SSE2 control and status word and the x87 control word have to preserved across function calls.
You are right. For correctness you need to preserve them. It would be nice to have an option to remove these save/restore when the user does not manipulate the control word or it manipulates it program wise.
BTW, the current code is already inconsistent because it is saving
the
SSE2 control world on amd64 but not on i386.
I didn't read in the SYSV I386 ABI document about the fact that it requires to preserve SSE2 stuff.
I think this is still the official SYSV i386 abi document:
http://www.sco.com/developers/devspecs/abi386-4.pdf
but it doesn't mention SSE at all, so it can be complete. Do you have any other reference?
What do you mean by "official SYSV i386 abi document"? I would think that the compiler vendors are more or less free to use whatever abi they deem appropriate. Of course they will try to be reasonable abi compatible with other compilers and between different releases of the same compiler. Perhaps compiler vendors in 1997 (the date from the linked document) tried to follow the linked document more or less, but I can't believe that this should be still true today, more than ten years later. So I fail to see the relevance of the linked paper and I think that referring to it is even slightly dangerous, because it seems to imply that there actually exists such a thing as an official SYSV i386 abi document. (Background: I regularly test the numeric_bindings library on different target platforms. It's pretty safe to use the fortran compiler from the same vendor and release as the c++ compiler, but there can occasionally be issues when mixing compilers from different versions or vendors. This is the reason why I don't believe that there should really exist an official i386 abi document.) Regards, Thomas
On Fri, Mar 25, 2011 at 1:04 PM, Thomas Klimpel
I think this is still the official SYSV i386 abi document:
http://www.sco.com/developers/devspecs/abi386-4.pdf
but it doesn't mention SSE at all, so it can be complete. Do you have any other reference?
What do you mean by "official SYSV i386 abi document"? I would think that the compiler vendors are more or less free to use whatever abi they deem appropriate. Of course they will try to be reasonable abi compatible with other compilers and between different releases of the same compiler. Perhaps compiler vendors in 1997 (the date from the linked document) tried to follow the linked document more or less, but I can't believe that this should be still true today, more than ten years later.
So I fail to see the relevance of the linked paper and I think that referring to it is even slightly dangerous, because it seems to imply that there actually exists such a thing as an official SYSV i386 abi document.
(Background: I regularly test the numeric_bindings library on different target platforms. It's pretty safe to use the fortran compiler from the same vendor and release as the c++ compiler, but there can occasionally be issues when mixing compilers from different versions or vendors. This is the reason why I don't believe that there should really exist an official i386 abi document.)
The key is SYSV; that ABI is not the official i386 abi for all platforms, but for a vendor-agreed ABI for System V derived unices (today that would be Solaris, mostly) on that architecture. Other unix-like operating systems (mostly Linux and BSD derived systems) usually followed the same API for compatiblity. With time it should have been expanded with support for additional functionality (MMX and SSE), and these extensions must be documented somewhere but I quick search doesn't find anything relevant. May be the unwritten convention is just to do what what the Linux ABI (mostly as defined by GCC) does. -- gpd
A stack can only be safely deallocated if either has been completely unwound or if it doesn't own any resources (note that is not enough to just destroy any stack allocated object). Context should either:
a) provide an interface to unwind the stack (by automatically trowing a special exception for example), or
After some thoughts: I can provide a function returning if the function has returned normaly (==finished) or was left by a jump. bool context::finished() const; I think that using an exception for stack unwinding is false because if can easly catched by try/catch(...) clause and then this code might not be give the exceution back/does arbitrary thing. I beleive it would be correct to use the unwind informations on the stack to do an stack unwinding (using DWARF-based unwinding). regards, Oliver -- GMX DSL Doppel-Flat ab 19,99 Euro/mtl.! Jetzt mit gratis Handy-Flat! http://portal.gmx.net/de/go/dsl
The first issue is that the destructor of the context class will (at least by default) happily destroy the stack. This unfortunately will leak any resource owned by any object on the stack; This is not unlike destroying an allocator before having destroyed all allocated object.
I want to add a small point. When I reviewed Boost.Context I had thought at the begging about it but then I realized that it is very problematic requirement. The only portable way to abort stack unwinding is to throw an exception and catch it at top level. context_1.jump_to(context_2); // throws something like fiber interrupted. // if stack is destroyed. Actually it is quite problematic, for example what if there is a code that does try { ... context_1.jump_to(context_2); ... } catch(...) { // Ignore all errors } The stack would not be unwinded and the destructor of the stack would be stuck in execution of current destroyed thread. Think of thread cancellation, for example on Linux the program below unwinds C++ stack only of the throw below exists, if it commented out the program aborts with ctor start FATAL: exception not rethrown If throw exists ctor start dtor As expected. FreeBSD it does not unwind the stack at all, and it is not required by the standard so you would see ctor start #include <pthread.h> #include <iostream> #include <unistd.h> void *thread(void *) { { struct test { test() { std::cerr << "ctor" << std::endl; } ~test() { std::cerr << "dtor" << std::endl; } } tester; std::cerr << "start" << std::endl; for(int i=0;i<1000;i++) { try { // canelation poit usleep(10000); } catch(...) { throw; // If commented progam // aborts } } std::cerr << "end" << std::endl; } } int main() { pthread_t p; pthread_create(&p,0,thread,0); usleep(1000000); pthread_cancel(p); pthread_join(p,0); return 0; } So leaving this to higher level abstraction would be correct because there is not simple or portable way to do this because you can't force program to ignore catch(...) {} My small comments. Artyom
On Fri, Mar 25, 2011 at 12:29 PM, Artyom <artyomtnk@yahoo.com> wrote:
The first issue is that the destructor of the context class will (at least by default) happily destroy the stack. This unfortunately will leak any resource owned by any object on the stack; This is not unlike destroying an allocator before having destroyed all allocated object.
I want to add a small point.
When I reviewed Boost.Context I had thought at the begging about it but then I realized that it is very problematic requirement.
The only portable way to abort stack unwinding is to throw an exception and catch it at top level.
context_1.jump_to(context_2); // throws something like fiber interrupted. // if stack is destroyed.
Actually it is quite problematic, for example what if there is a code that does
try { ... context_1.jump_to(context_2); ... } catch(...) { // Ignore all errors }
The stack would not be unwinded and the destructor of the stack would be stuck in execution of current destroyed thread.
There are two solutions: a) require that that the unwind exception not be swallowed. It can either be made unswallowable by having a trowing destructor (IIRC this is how GCC implements the unwinding exception) or simply mark the context as being in the unwinding and assert at next jump_to from that context. b) do not unwind in the destructor but assert that the stack is unwound. A separate terminate() function can be used to unwind the stack. This function would restore the context, try to unwind it and return to the terminator context if the stack is unwound safely. When control reaches back to the terminator context, directly or indirectly, if the stack has not been unwound an exception is thrown from terminate(). I opted for option a on my coroutine implementation, but now I think it is too harsh and I favor option b better. Option b is similar to std::thread which requires a joinable thread to be joined before calling the destructor. -- gpd
participants (4)
-
Artyom -
Giovanni Piero Deretta -
Oliver Kowalke -
Thomas Klimpel