Philipp, All,
Given the lack of traffic on the reflector, I am assuming everyone is in
agreement with Philipp and I continuing to thrash out this implementation.
I would be okay with saying that If an event list is already destroyed,
when a sensitive process is triggered (and therefore needs to access this
list), the behaviour shall be undefined.
Note that according to the fine print of the LRM, this gives a smart
implementation the choice of detecting and reporting an error.
Now for matching the LRM with the implementation. The wait/next_trigger
methods have an argument of type sc_event_and/or_list, which is currently
an implementation-defined class name. The proposal is to promote
sc_event_and/or_list to standard classes. We then also need to introduce
sc_event_and/or_expr classes in order to provide the & | operators used in
calls to wait/next_trigger. I propose that sc_event_and/or_expr should
become implementation-defined "daggered" names, just as
sc_event_and/or_list are at present.
Note that this approach is (I think) backward compatible with 1666-2005,
but is not backward compatible with any application code that abused the
sc_event_and/or_list classes from the 1666-2005 standard (which forbade
their explicit use in an application).
John A
From:
"Philipp A. Hartmann" <philipp.hartmann@offis.de>
To:
John Aynsley <john.aynsley@doulos.com>
Cc:
SystemC P1666 Technical <systemc-p1666-technical@eda.org>, "Jeremiassen,
Tor" <tor@ti.com>
Date:
13/10/2010 19:07
Subject:
Re: sc_event_and/or_list
John, All,
John found a problem in the prototypical implementation of a possible
new sc_event_list interface. The detection of prematurely destroyed
event lists is not reliably possible in all cases:
If an event list is used as a member of a module, its lifetime may
correctly end during the destruction of the module hierarchy, although
there are still child processes of the current module waiting for it.
Detecting (and ignoring) this corner-case is quite difficult for
similar reasons to the sc_is_running() return value during the
destruction of the system: The event list is already destroyed before
the ~sc_module() destructor is called (where process cleanup could occur).
Consequently, my suggestion would be that an implementation should not
be obliged to detect insufficient lifetimes of passed event lists. If
an event list is already destroyed, when a sensitive process is
triggered (and therefore needs to access this list), the behaviour shall
be undefined.
A workaround to at least detect most prematurely deleted event lists
would be to perform the check only within running processes. This
catches at least the "next_trigger( all_events() )" case:
void
sc_event_list::report_premature_destruction() const
{
if( sc_get_current_process_handle().valid() ) {
sc_assert( false && "sc_event_list prematurely destroyed" );
}
}
Malicious users could of course still (e.g. manually) delete event lists
during the update() phase or between calls to sc_start().
What do you think?
Philipp
On 13/10/10 18:20, Philipp A. Hartmann wrote:
> As a quick follow-up:
>
> Currently, the OSCI-kernel does not delete (or at least kill() remove
> from internal queues) any child-process of destroyed modules. This is
> in fact already a problem, since you can easily do something like
>
> SC_CTOR( some_module )
> {
> sub_module sub("sub") // on stack, instead of member
> }
> // sub destroyed too early, processes in "sub" will crash,
> // since their parent object is lost
>
> This has been asked several times in the user-fora.
> Maybe, we should define this behaviour as well?
>
> One soulution could be to remove child processes upon destruction of a
> module. (what about other child objects?) Child processes are likely to
> break, when their module ceases to exist anyhow.
>
> What do you think?
> Can we leave this as a pure implementation issue?
>
> Thanks,
> Philipp
>
> On 13/10/10 17:40, Philipp A. Hartmann wrote:
>> John,
>>
>> Yes, I've seen this with member-lists, when sc_main exits and objects
>> (and their member lists) are destroyed, but processes are still waiting
>> for them. I hoped, that you wouldn't notice ... ;-)
>>
>> Something like the following shows the problem:
>>
>> SC_MODULE(module)
>> {
>> sc_event e;
>> sc_event_and_list list1;
>> sc_process_handle h;
>>
>> SC_CTOR(module)
>> : list1( e )
>> {
>> SC_THREAD(do_it);
>> h = sc_get_last_created_process_handle();
>> }
>> void do_it()
>> {
>> wait( list1 ); // wait forever
>> }
>> };
>>
>> int sc_main( int, char*[] )
>> {
>> module m("m");
>> sc_start();
>>
>> // reset (only) process:
>> // m.h.reset();
>>
>> std::cout << "sc_main exiting..." << std::endl;
>> return 0;
>> }
>>
>> Do you see this in other cases as well?
>>
>> The above corner case looks like a bug in the kernel, which does not
>> clean up processes properly, though. You can currently ignore it, if
it
>> occurs after the 'sc_main exiting...' message. Local variables in
>> SC_THREAD processes should not have this problem (since they are not
>> destroyed).
>>
>> Another possibility is to reset() the processes manually, as outlined
in
>> the above code.
>>
>> Of course, you can also change sc_event.cpp to do nothing in
>> sc_event_list::report_premature_deletion().
>>
>> Hope that helps,
>> Philipp
>>
>> On 13/10/10 17:24, john.aynsley@doulos.com wrote:
>>> Philipp,
>>>
>>> With the new sc_event.* files, with pretty much any event list
>>> manipulation, I get:
>>>
>>> Fatal: (F4) assertion failed: false && "sc_event_list prematurely
>>> destroyed"
>>> In file: ../../../../src/sysc/kernel/sc_event.cpp:479
>>>
>>> I simply copied the files on top of the previous patched version 8=(
>>>
>>> John A
>>>
>>>
>>>
>>> From:
>>> "Philipp A. Hartmann" <philipp.hartmann@offis.de>
>>> To:
>>> john.aynsley@doulos.com
>>> Cc:
>>> systemc-p1666-technical@eda.org, "Jeremiassen, Tor" <tor@ti.com>
>>> Date:
>>> 12/10/2010 18:08
>>> Subject:
>>> Re: sc_event_and/or_list
>>>
>>>
>>>
>>> John,
>>>
>>> sorry for the delay. I finally found some time to look at this.
>>> Comments embedded below.
>>>
>>> On 12/10/10 16:22, john.aynsley@doulos.com wrote:
>>> [snip]
>>>
>>>> Using Philipp's prototype we can do some cool stuff with event lists,
>>> for
>>>> example:
>>> [snip some code examples]
>>>
>>>> sc_event_and_list list3 = list2 & e2; // Caveat - modifies list2
>>>> wait( list3 );
>>>
>>> I agree, that this is an unexpected side-effect. I would suggest to
add
>>> &= and |= operators to the lists to modify lists in place, though:
>>>
>>> class sc_event_or_list {
>>> // ...
>>> sc_event_or_list& operator |= ( const sc_event& );
>>> sc_event_or_list& operator |= ( const sc_event_or_list & );
>>>
>>> sc_event_or_expr operator | ( const sc_event& ) const;
>>> sc_event_or_expr operator | ( const sc_event_or_list& ) const;
>>> };
>>>
>>> (sc_event_or_expr is an implementation defined helper class, as you
>>> suggested, see below).
>>>
>>>> However, because the ctor sc_event_and_list(const sc_event&) is
>>> protected,
>>>> we cannot do
>>>>
>>>> sc_event_and_list list5 = e3;
>>>>
>>>> I think it would be nice to be able to initialize a list with a
single
>>>> event, so I propose we make this ctor public (but without putting the
>>>> auto_delete argument in the LRM)
>>>
>>> I'm not sure, if this is really required. With the '&|='-operators,
the
>>> same could be written as:
>>>
>>> sc_event_and_list list5;
>>> list5 &= e3;
>>>
>>> Nevertheless it seems not to cause real harm in the usual case. But
>>> note, that for your desired syntax these constructors can not be
>>> 'explicit', which may lead to ambiguities if some functions are
>>> overloaded for sc_event_and_list and sc_event_or_list but not sc_event
>>> (not in the kernel, though).
>>>
>>>> Also, we cannot do
>>>>
>>>> sc_event_and_list list6 = list3 & list4;
>>>>
>>>> because we have no sc_event_and_list& operator & ( const
>>>> sc_event_and_list& );
>>>> I propose we add this operator
>>>
>>> Indeed, this would be very useful. Implemented in the attached
quick'n
>>> dirty prototype as well.
>>>
>>>> Finally, note that
>>>>
>>>> list2 = list2 & e1;
>>>>
>>>> is equivalent to
>>>>
>>>> list2 & e1;
>>>>
>>>> the latter being permitted because operator& works through a
>>> side-effect.
>>>> Would this be a reason for introducing a separate class for
>>>> user-instantiated event lists?
>>>
>>> Yes, this is a strong reason for adding implementation-defined
>>> expression helper classes. They can be implemented fairly easily
around
>>> a temporary (auto-deleted) sc_event_list, which gives up ownership
upon
>>> conversion to a list object.
>>>
>>> I have attached updated sc_event.(h,cpp) classes, which implement
nearly
>>> all of the proposed extensions:
>>>
>>> - construction from single events
>>> - expression classes (sc_event_and_expr, sc_event_or_expr)
>>> - list concatenation ( list1 & list2 )
>>> - operator &=, |=
>>> - size()
>>> - swap()
>>> - move-semantics for temporary lists (implementation detail)
>>>
>>> The implementation is backwards-compatible, even regarding the
>>> 'reference hack' and can detect premature deletion (or modification)
of
>>> busy event lists (i.e. with currently sensitive processes).
>>>
>>> The new classes should work with the earlier proposed 'const patch',
>>> where wait() and next_trigger() take const list-references. This
>>> earlier patch (against OSCI PoC version 2.3.10dec09_beta) can be found
>>> in the Twiki [1].
>>>
>>> Greetings from Oldenburg,
>>> Philipp
>>>
>>> [1]
>>>
http://www.eda.org/twiki/pub/Main/PhilippHartmann/0001-make-event-handling-const.patch
>>>
>>>
>>
>>
>
>
-- Philipp A. Hartmann Hardware/Software Design Methodology Group OFFIS Institute for Information Technology R&D Division Transportation · FuE-Bereich Verkehr Escherweg 2 · 26121 Oldenburg · Germany Phone/Fax: +49-441-9722-420/282 · PGP: 0x9161A5C0 · http://www.offis.de/ [attachment "sc_event.cpp" deleted by John Aynsley/doulos] -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.Received on Wed Oct 20 04:15:23 2010
This archive was generated by hypermail 2.1.8 : Wed Oct 20 2010 - 04:15:24 PDT