Re: [boost] [context] ready

Subject: Re: [boost] [context] ready
From: TONGARI (tongari95_at_[hidden])
Date: 2011-11-09 04:36:32

2011/11/9 Oliver Kowalke <oliver.kowalke_at_[hidden]>

>
> > Your work is appreciated, and I really want to try it, unfortunately it
> > doesn't support MinGW yet.
>
> maybe MinGW is supported later (requires only to adapt the Windows asm to
> GAS style + entry in Jamfile).
>

Actually I've tried some MASM/GAS convert tools but with no luck :/

> > 1) The doc says suspend() throws nothing, but it does throw
> > ex_unwind_stack for unwinding.
> > Maybe it'd be better to make ex_unwind_stack part of API, and let the
> > user catch and rethrow.
>
> ex_unwind_stack is swallowed by boost::context - it is required in order
> to implement expilict stack unwinding.
> It is important that the user dosn't swallow this exception!
> I think the best would be that the user dosn't know the type of this
> exception (unnamed exception?) - not sure if I should supress this info or
> note it explicitly in the docu.
>

Indeed the user should not swallow it, but there's no reason to prohibit
the user from using catch(...) as well, so why not let the user catch and
rethrow ex_unwind_stack explicitly for stack unwinding?

> 2) Many functions seem unnecessarily virtual, IMO, only dtor and exec need
> > to be virtual.
> > icontext could be replaced with platform-specific context_base
> > without Allocator stored, and you could store Allocator in context_object
> > instead, passing it to templated methods of context_base when needed
> (e.g.
> > on constructon & cleanup).
>
> boost::context should be independed from the used Allocator. That requires
> that the pointer to the impl held inside boost::context doesn't contain the
> allocator type in its class declaration. Thatswhy icontext was introduced.
> context_base uses the allocator in order to allocate/deallocate the stack
> as well aplly it to fcontext_t.
>
> But maybe you can send me your solution so I can take alook at it?
>

My solution is: remove icontext, use non-templated context_base directly.
So it looks like:

//////////////////
// platform-specific
class context_base
{
public:

    typedef intrusive_ptr< context_base >   ptr_t;

    template<class Allocator>
    context_base( Allocator& alloc, ...)
    { ... }

    template<class Allocator>
    void cleanup(Allocator& alloc)
    {
        // as in original dtor
    }

    virtual ~context_base() = 0;

    bool unwind_requested() const
    { ... }

    bool is_complete() const
    { ... }

    bool is_started() const
    { ... }

    bool is_resumed() const
    { ... }

    bool is_running() const
    { ... }

    void * start()
    { ... }

    void * resume( void * vp)
    { ... }

    void * suspend( void * vp)
    { ... }

    void unwind_stack()
    { ... }

    virtual void exec() = 0;
};

template< typename Fn, typename Allocator >
class context_object : public context_base
{
private:
    Fn  fn_;
    Allocator alloc_;
public:
    context_object(...)
    { ... }

    ~context_object()
    {
        cleanup(alloc_);
    }

    void exec()
    { fn_(); }
};
//////////////////

Something I missed?