Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Runtime support for watchdogs #177

Merged
merged 57 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
d28b56d
saving changes made to runtime
Jan 5, 2023
37a9604
removed watchdog handler from reaction struct, not needed
Jan 6, 2023
11dc26d
defined watchdog_t struct
Jan 8, 2023
96da894
added additional changes to struct definitions
Jan 18, 2023
90a0a9b
added questions about watchdog start and stop
Jan 18, 2023
050abfa
saving changes
Jan 25, 2023
0820da9
save before merge
Feb 1, 2023
92160a8
include all
Feb 1, 2023
04173e9
committing types
Feb 10, 2023
7fd67a5
Merge remote-tracking branch 'origin/main' into watchdogs
Feb 10, 2023
6dff526
fix build problems associated w merge
Feb 17, 2023
0f28759
should fix issues with build after submodule update
Feb 17, 2023
59dfc4f
fixed c compile errors
Feb 20, 2023
9a40303
fixed c compile problems
Feb 20, 2023
b4b70c5
saving reactor common c before submodule update
Feb 21, 2023
c974d4d
Merge branch 'watchdogs' of https://github.com/lf-lang/reactor-c into…
Feb 21, 2023
3358cd1
saving actual fixes to c compilation issues
Feb 21, 2023
4677a37
Merge branch 'temp-branch' into watchdogs
Feb 21, 2023
38aec13
fixed
Feb 21, 2023
c643234
tried applying fix for non-threaded runtime
Feb 26, 2023
b97d084
fixed merge conflict from detached head
Feb 26, 2023
c00691a
Merge remote-tracking branch 'origin/main' into watchdogs
Feb 26, 2023
8a8827f
fixed some compile issues w non-threaded
Mar 3, 2023
50ef0d2
saving fix to watchdogs on unthreaded
Mar 9, 2023
742f294
Update lingua-franca-ref.txt
lhstrh Mar 9, 2023
faaf56b
removed fixmes
Mar 9, 2023
4fe27cc
removed fixmes
Mar 9, 2023
f5825b4
fixed ccpp test issue?
Mar 9, 2023
82afe9b
Merge branch 'main' into watchdogs
Benichiwa Mar 12, 2023
ff3c84c
removed unnecessary ifdef endif
Mar 15, 2023
8ca094f
fixed some code review suggestions
Mar 20, 2023
10c275b
Merged main into watchdogs
edwardalee Mar 25, 2023
e516564
Made watchdog function match thread function signature
edwardalee Mar 25, 2023
b62683e
Comment formatting only
edwardalee Mar 25, 2023
24c0dc7
Made watchdog function conform with thread function
edwardalee Mar 25, 2023
e24e3bc
Merge pull request #184 from lf-lang/watchdogs-eal
Benichiwa Mar 31, 2023
ccb0a3c
updating reactor-c
Apr 21, 2023
9b6c7dc
fixed merge conflicts
Apr 21, 2023
7dcdb7d
fixed problem from reactor-commonc merge
Apr 21, 2023
9ba9817
Merge branch 'main' into watchdogs
Benichiwa Apr 21, 2023
2c7a514
Added LF_SOURCE_DIRECTORY and LF_FILE_SEPARATOR
edwardalee Apr 21, 2023
05b6e96
Cleaned up docs for doxygen
edwardalee Apr 21, 2023
795d38b
Cleaned up docs and const args
edwardalee Apr 21, 2023
24e1dc0
Added cmake file for wave reader
edwardalee Apr 21, 2023
cee63e1
const modifier to suppress warnings
edwardalee Apr 23, 2023
a48b022
Moved \#define from platform.h to tag.h because platform.h is no long…
edwardalee Apr 20, 2023
02cbf70
Moved memory leak check to account for persistent tokens
edwardalee Apr 26, 2023
2f51b30
Add CMake definition to find FEDERATED_AUTHENTICATED
Apr 15, 2023
436decc
Change ci.yml point
Apr 15, 2023
c985c6a
Try openssl/hmac.h first
Apr 16, 2023
775b27a
Revert "Try openssl/hmac.h first"
Apr 16, 2023
eede0fa
Fix ci tests
Apr 21, 2023
aeff26c
Add openssl library links to reactor-c/core
Apr 21, 2023
670f634
Add mac options
Apr 22, 2023
9ae8632
Uncommented code tof fix memory leak
edwardalee Apr 25, 2023
5427b5d
Removed incorrect free
edwardalee Apr 25, 2023
4265119
Resurrected lf_set_array
edwardalee Apr 26, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions core/reactor.c
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,9 @@ int lf_reactor_c_main(int argc, const char* argv[]) {
_lf_execution_started = true;
_lf_trigger_startup_reactions();
_lf_initialize_timers();

_lf_initialize_watchdog_mutexes();

// If the stop_tag is (0,0), also insert the shutdown
// reactions. This can only happen if the timeout time
// was set to 0.
Expand Down
52 changes: 52 additions & 0 deletions core/reactor_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -1316,18 +1316,70 @@ bool _lf_check_deadline(self_base_t* self, bool invoke_deadline_handler) {
return false;
}

#ifdef LF_THREADED

void* run_watchdog(watchdog_t* watchdog) {
watchdog->thread_active = true;
self_base_t* base = watchdog->base;

while (lf_time_physical() < watchdog->expiration) {
interval_t T = watchdog->expiration - lf_time_physical();
lf_mutex_unlock(&(base->watchdog_mutex));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesnt seem right. The first iteration of this while loop you are not holding the mutex and can thus not unlock it. Please document how the interaction between this thread running a single watchdog and the worker threads executing reactions should look like.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesnt seem right. The first iteration of this while loop you are not holding the mutex and can thus not unlock it.

Yep, should be fixed now.

Please document how the interaction between this thread running a single watchdog and the worker threads executing reactions should look like.

Not sure if there is a better way of drawing this out:

  • _lf_invoke_reaction locks the mutex if the reaction is part of a reactor that contains a watchdog
  • _lf_invoke_reaction unlocks the mutex after invoking the reaction
  • _lf_watchdog_start unlocks the mutex only after creating the thread for the watchdog
  • run_watchdog locks the mutex that was just unlocked by _lf_watchdog_start
  • run_watchdog unlocks->locks the mutex inside the while loop so that other reactions in the reactor can run while the thread is asleep.
  • run_watchdog unlocks the mutex after executing the watchdog function

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to have it represented as a sequence. In what order to you expect this to happen. I think there is some flaw in your logic here. The watchdog mutex should be held whenever a reaction within that reactor is executed. I don't think it makes sense to unlock that mutex when you call _lf_watchdog_start from inside a reaction. Remember that a mutex has to unlocked by the same thread that locks it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay. My thought process was the following:
Thread 1: If reaction is in a reactor that contains a watchdog, lock mutex - lf_invoke_reaction.
Thread 1: Execute reaction.
Thread 1: If reaction body calls _lf_watchdog_start , start thread 2.
Thread 1: Inside _lf_watchdog_start, unlock mutex.
Thread 2: acquire mutex, set expiration, and unlock mutex
Thread 2: sleep
Thread 1: Inside _lf_watchdog_start- if the mutex has been unlocked, lock mutex
Thread 1: Unlock mutex inside lf_invoke_reaction.
Thread 2: Once done sleeping, lock mutex, and either reset expiration or execute watchdog function.

In hindsight, I'm now realizing that I need to reset the sleep each time, or I will get undesired behavior past the first watchdog function call.

lf_nanosleep(T);
lf_mutex_lock(&(base->watchdog_mutex));
}


watchdog_function_t watchdog_func = watchdog->watchdog_function;
(*watchdog_func)(watchdog);
lf_mutex_unlock(&(base->watchdog_mutex));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you return here, the thread_active field is still true. So there wont be another thread created. Also, shouldnt the thread keep looping inside this function and wait for a watchdog reset?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you return here, the thread_active field is still true. So there wont be another thread created.

This has been changed so that the thread_active field is set to false.

Also, shouldnt the thread keep looping inside this function and wait for a watchdog reset?

Calling lf_watchdog_start should restart the watchdog and create another thread after the previous one has ended. If we want to have a watchdog that keeps running without a restart after being executed, I can put the function inside the loop and change the conditions so that it continues looping, but that was not my understanding of the intended behavior.

Copy link
Collaborator

@erlingrj erlingrj Mar 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty sure we wouldnt want that. Creating a thread has a significant overhead. It would be better to have a single thread per watchdog.

}

void _lf_watchdog_start(watchdog_t* watchdog, interval_t additional_timeout) {
self_base_t* base = watchdog->base;

watchdog->expiration = lf_time_logical() + watchdog->min_expiration + additional_timeout;


if (!watchdog->thread_active) {
lf_thread_create(&(watchdog->thread_id), run_watchdog, watchdog);
}

watchdog->thread_active = false;
erlingrj marked this conversation as resolved.
Show resolved Hide resolved
}

//FIXME: modif4watchdogs
void _lf_watchdog_stop(watchdog_t* watchdog) {

}
#endif

/**
* Invoke the given reaction
*
* @param reaction The reaction that has just executed.
* @param worker The thread number of the worker thread or 0 for unthreaded execution (for tracing).
*/
void _lf_invoke_reaction(reaction_t* reaction, int worker) {

#ifdef LF_THREADED
if (&(((self_base_t*) reaction->self)->watchdog_mutex) != NULL) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesnt seem right. I think you rather want to test the value of that field, not its address. The address of the watchdog_mutex field is some offset + the address of the reaction->self struct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think that was actually causing an issue once I fixed the race condition associated with 'thread_active' field race condition mentioned below. I now use a boolean field added to the self_base_t struct indicating whether or not the mutex has been initialized.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think have a lf_mutex_t *, i.e. a pointer to a mutex, would be better. Then its either set to NULL, or initialized with lf_mutex_create during init

lf_mutex_lock(&(((self_base_t*) reaction->self)->watchdog_mutex));
}
#endif

tracepoint_reaction_starts(reaction, worker);
((self_base_t*) reaction->self)->executing_reaction = reaction;
reaction->function(reaction->self);
((self_base_t*) reaction->self)->executing_reaction = NULL;
tracepoint_reaction_ends(reaction, worker);


#ifdef LF_THREADED
if (&(((self_base_t*) reaction->self)->watchdog_mutex) != NULL) {
lf_mutex_unlock(&(((self_base_t*) reaction->self)->watchdog_mutex));
}
#endif
}

/**
Expand Down
2 changes: 2 additions & 0 deletions core/threaded/reactor_threaded.c
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,8 @@ int lf_reactor_c_main(int argc, const char* argv[]) {
// it can be probably called in that manner as well).
_lf_initialize_start_tag();

_lf_initialize_watchdog_mutexes();

start_threads();

lf_mutex_unlock(&mutex);
Expand Down
11 changes: 11 additions & 0 deletions include/api/api.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,17 @@ trigger_handle_t lf_schedule_value(void* action, interval_t extra_delay, void* v
*/
bool lf_check_deadline(void* self, bool invoke_deadline_handler);

#ifdef LF_THREADED
/**
* Begin the watchdog.
*
* @param watchdog The watchdog to be started.
* @param additional_timeout The timeout to be added to the minimum
* experiation of the watchdog.
**/
void lf_watchdog_start(watchdog_t* watchdog, interval_t additional_timeout);
#endif

/**
* Compare two tags. Return -1 if the first is less than
* the second, 0 if they are equal, and +1 if the first is
Expand Down
27 changes: 27 additions & 0 deletions include/core/lf_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,13 @@ typedef pqueue_pri_t index_t;
*/
typedef void(*reaction_function_t)(void*);

/**
* Watchdog function type. The argument passed to one of
* these watchdog functions is a pointer to the self struct
* for the reactor.
*/
typedef void(*watchdog_function_t)(void*);

/** Trigger struct representing an output, timer, action, or input. See below. */
typedef struct trigger_t trigger_t;

Expand Down Expand Up @@ -210,6 +217,22 @@ struct event_t {
event_t* next; // Pointer to the next event lined up in superdense time.
};

/** Typdef for watchdog_t struct, used to call watchdog handler. */
typedef struct watchdog_t watchdog_t;

#ifdef LF_THREADED
/** Watchdog struct for handler. */
struct watchdog_t {
struct self_base_t* base; // The reactor that contains the watchdog.
instant_t expiration; // The expiration instant for the watchdog. (Initialized to NEVER)
interval_t min_expiration; // The minimum expiration interval for the watchdog.
lf_thread_t thread_id; // The thread that the watchdog is meant to run on.
bool thread_active; // Boolean indicating whether or not thread is active.
watchdog_function_t watchdog_function; // The function/handler for the watchdog.
};
#endif


/**
* Trigger struct representing an output, timer, action, or input.
*/
Expand Down Expand Up @@ -279,6 +302,10 @@ typedef struct allocation_record_t {
typedef struct self_base_t {
struct allocation_record_t *allocations;
struct reaction_t *executing_reaction; // The currently executing reaction of the reactor.
#ifdef LF_THREADED
lf_mutex_t watchdog_mutex; // The mutex for this reactor to be acquired before reaction
// invocation.
#endif
#ifdef MODAL_REACTORS
reactor_mode_state_t _lf__mode_state; // The current mode (for modal models).
#endif
Expand Down
39 changes: 39 additions & 0 deletions include/core/reactor.h
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,11 @@ void _lf_initialize_timers(void);
*/
void _lf_trigger_startup_reactions(void);

/**
* Function to initialize mutexes for watchdogs
*/
void _lf_initialize_watchdog_mutexes(void);


/**
* Function (to be code generated) to terminate execution.
Expand Down Expand Up @@ -553,5 +558,39 @@ void _lf_fd_send_stop_request_to_rti(void);
*/
bool _lf_check_deadline(self_base_t* self, bool invoke_deadline_handler);

#ifdef LF_THREADED
/**
* Function to start the watchdog.
*
* @param watchdog The watchdog to be started
* @param additional_timeout Additional timeout to be added to the watchdog's
* minimum expiration.
**/
void _lf_watchdog_start(watchdog_t* watchdog, interval_t additional_timeout);
#endif

/**
* These functions must be implemented by both threaded and unthreaded
* runtime. Should be routed to appropriate API calls in platform.h
*/

/**
* @brief Notify other threads of new events on the event queue.
*
*/
void _lf_notify_of_event();

/**
* @brief Enter critical section. Must be paired with a
* `_lf_critical_section_exit()`
*
*/
void _lf_critical_section_enter();

/**
* @brief Leave critical section
*/
void _lf_critical_section_exit();

#endif /* REACTOR_H */
/** @} */
5 changes: 5 additions & 0 deletions include/core/reactor_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ trigger_handle_t _lf_insert_reactions_for_trigger(trigger_t* trigger, lf_token_t
void _lf_advance_logical_time(instant_t next_time);
trigger_handle_t _lf_schedule_int(lf_action_base_t* action, interval_t extra_delay, int value);
bool _lf_check_deadline(self_base_t* self, bool invoke_deadline_handler);
#ifdef LF_THREADED
void* run_watchdog(watchdog_t* watchdog);
void _lf_watchdog_start(watchdog_t* watchdog, interval_t additional_timeout);
void _lf_watchdog_stop(watchdog_t* watchdog);
#endif
void _lf_invoke_reaction(reaction_t* reaction, int worker);
void schedule_output_reactions(reaction_t* reaction, int worker);
int process_args(int argc, const char* argv[]);
Expand Down
6 changes: 6 additions & 0 deletions lib/schedule.c
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,9 @@ trigger_handle_t lf_schedule_value(void* action, interval_t extra_delay, void* v
bool lf_check_deadline(void* self, bool invoke_deadline_handler) {
return _lf_check_deadline((self_base_t*)self, invoke_deadline_handler);
}

#ifdef LF_THREADED
void lf_watchdog_start(watchdog_t* watchdog, interval_t additional_timeout) {
return _lf_watchdog_start(watchdog, additional_timeout);
}
#endif
2 changes: 1 addition & 1 deletion lingua-franca-ref.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
master
watchdogs
1 change: 1 addition & 0 deletions test/src_gen_stub.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ bool _lf_trigger_shutdown_reactions() { return true; }
void _lf_set_default_command_line_options() {}
void _lf_trigger_startup_reactions() {}
void _lf_initialize_timers() {}
void _lf_initialize_watchdog_mutexes() {}
void logical_tag_complete(tag_t tag_to_send) {}