Killing a process with an included sc_event_[and|or]_list

[Stephan Gerth]

In reference to http://forums.accellera.org/topic/6218-wait-is-not-allowed-inside-run_phase/ I created a small demo which exhibits the same behaviour:

1. create a process which waits on an event list which resides on its stack (and waits there forever)
2. create a second process which, shortly after starting the simulation, will kill() the other process.

This creates the following fatal message:

Quote

Fatal: (F565) invalid use of sc_(and|or)_event_list: list prematurely destroyed
In file: ../../../systemc-2.3.3/src/sysc/kernel/sc_event.cpp:688
In process: top.thread_2 @ 1 ns

Info: (I99) simulation aborted

This begs the question if this is intended behaviour or is some kind of bug: I guess in principle it should be possible to kill() a thread which is waiting on an event list. But if the event list object resides on its stack the above message is printed. If the objects is e.g. an object member, the simulation runs without error.

 

Demo source:

#include <systemc.h>

SC_MODULE(top) {
public:
	sc_event e;

	void thread_1() {
		wait(1,SC_NS); // be sure that thread_2 is already waiting
		t2_hdl.kill();
	}

	void thread_2() {
		sc_event_or_list terminated_events;
		terminated_events |= e;
		sc_core::wait(terminated_events); // will never finish
	}

	SC_CTOR(top)  {
		SC_THREAD(thread_1);
		SC_THREAD(thread_2);
		t2_hdl = sc_get_current_process_handle();
	}

	sc_process_handle t2_hdl;
};

int sc_main(int argc, char **argv) {
	top i_top("top");
	sc_start();
	return 0;
}

 

[Roman Popov]

6 hours ago, StS said:

This begs the question if this is intended behaviour or is some kind of bug: I guess in principle it should be possible to kill() a thread which is waiting on an event list. But if the event list object resides on its stack the above message is printed. If the objects is e.g. an object member, the simulation runs without error.

Here is comment from Accellera SystemC 2.3.3:

void
sc_event_list::report_premature_destruction() const
{
    // TDB: reliably detect premature destruction
    //
    // If an event list is used as a member of a module,
    // its lifetime may (correctly) end, although there
    // are processes currently 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 module
    // hierarchy.
    //
    // Ignoring the lifetime checks for now, if no process
    // is currently running (which is only part of the story):

I don't know what "TDB" means here, but it seems like the case when sc_event_list resides on SC_THREAD stack was not considered here.

So we have two options how to fix this:

1. Remove report_premature_destruction check.
2. Register sc_event_lists inside process handler, and notify them when thread is terminated so it longer waits for event.

[Stephan Gerth]

I'm a bit in favour of removing this check, albeit I'm not completely clear about the reasons why it was introduced at some point. If an event list gets destroyed by some means (module gets deleted, thread gets killed, explicit deletion via pointer, ...) any process which was still waiting on it should keep waiting (forever). The destruction of the event list should not raise a notification to waiting processes as this was probably not the expectation of them being notified.

[Philipp A Hartmann]

14 hours ago, Roman Popov said:

I don't know what "TDB" means here, but it seems like the case when sc_event_list resides on SC_THREAD stack was not considered here.

I did write the above comment and the case where a (e.g. temporary) list lives inside a thread is certainly one of the reasons why we need a check for premature destruction.  Due to the nature how event lists are internally handled, we will run into a memory corruption if list is destroyed while a process is still waiting for it.

What was not considered is the explicit kill() of a process that is current waiting on such a list.

The easiest fix would be to cancel (dynamic) sensitivity before doing the stack unwinding when killing the process.

@Stephan Gerth: Can you submit a pull-request with your example above as testcase against the SystemC regressions repository?

[Stephan Gerth]

On 2/1/2019 at 10:46 AM, Philipp A Hartmann said:

@Stephan Gerth: Can you submit a pull-request with your example above as testcase against the SystemC regressions repository?

Done.

[Philipp A Hartmann]

18 hours ago, Stephan Gerth said:

Done.

Thanks! I can reproduce the behavior and verified that removing the dynamic sensitivity in sc_thread_process::kill_process fixes the issue:

void sc_thread_process::kill_process(sc_descendant_inclusion_info descendants )
{
    // ...
    if ( sc_is_running() && m_has_stack )
    {
        m_throw_status = THROW_KILL;
        m_wait_cycle_n = 0;
        remove_dynamic_events(); // <-- add this line to avoid the exception
        simcontext()->preempt_with(this);
    }
    // ...
}

I'm not sure, if it is necessary to do the same for the static sensitivity.  At least I haven't come up with a similar scenario, where the error is actually "incorrect".