Opened 9 years ago

Last modified 9 years ago

#9525 new Bugs

Assert with program termination due to context<...>() call in simple_state's entry action is incorrect

Reported by: anonymous Owned by: Andreas Huber
Milestone: To Be Determined Component: statechart
Version: Boost 1.49.0 Severity: Problem
Keywords: Cc:

Description

#include <boost/statechart/state_machine.hpp>
#include <boost/statechart/simple_state.hpp>

namespace sc = boost::statechart;

struct state;
struct fsm : sc::state_machine<fsm, state> {
    void test() {}
};

struct state : sc::simple_state<state, fsm> {
    state() { context<fsm>().test(); }
};

int main() {
    fsm test;
    test.initiate();
}

statechart_test: /usr/include/boost/statechart/simple_state.hpp:682: static OtherContext& boost::statechart::simple_state<MostDerived, Context, InnerInitial, historyMode>::context_impl_other_context::context_impl(State&) [with OtherContext = fsm; State = boost::statechart::simple_state<state, fsm>; MostDerived = state; Context = fsm; InnerInitial = boost::mpl::list<mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>; boost::statechart::history_mode historyMode = (boost::statechart::history_mode)0u]: Assertion `get_pointer( stt.pContext_ ) != 0' failed.
Аварийный останов

It's not correct to throw uncatchable surprises like asserts. It's especially incorrect when your library is used in the embedded software.

Not to mention this isn't reflected anywhere in the documentation.

Please change this to exception throwing, or better, find a way to detect calls like that at compile time. But don't use asserts. That's just bad treatment of your users.

Change History (3)

in reply to:  description comment:1 by Andreas Huber, 9 years ago

Replying to anonymous:

It's not correct to throw uncatchable surprises like asserts. It's especially incorrect when your library is used in the embedded software.

Not to mention this isn't reflected anywhere in the documentation.

Do you use std::vector? If yes, I'm wondering what you get when you dereference the return value of std::vector::end()? Last time I accidentally did that I got an assertion in DEBUG mode. I RELEASE mode, on most platforms dereferencing the return value of std::vector::end() does the wrong thing and return the contents of an invalid memory location. IIRC, the standard just says that the iterator you get from vector::end() must not be dereferenced.

I would argue that it's an almost identical situation with the simple_state::context() function. The reference docs say that you can't call this function in the constructor of a simple_state subclass:

<http://www.boost.org/doc/libs/1_55_0/libs/statechart/doc/reference.html#ClassTemplatesimple_state>

Last edited 9 years ago by Andreas Huber (previous) (diff)

comment:2 by anonymous, 9 years ago

It says "must not call", but it doesn't say *anything* about getting an assert and the program termination when you make a mistake (e.g. add context() call to a simple_state constructor but forget to change simple_state into state). Is this really the only way you can say "you've made a mistake" here?

in reply to:  2 comment:3 by anonymous, 9 years ago

Replying to anonymous:

It says "must not call", but it doesn't say *anything* about getting an assert and the program termination when you make a mistake (e.g. add context() call to a simple_state constructor but forget to change simple_state into state).

IIRC, neither do the standard docs for std::vector::end.

Is this really the only way you can say "you've made a mistake" here?

The only other thing that comes to mind is throwing an exception, but exceptions have the problem that they can be ignored. Calling context() from a simple_state constructor makes the program broken beyond any hope of recovery. If you have any other suggestions, I'm all ears.

Note: See TracTickets for help on using tickets.