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

Critical section functions as part of the public API #142

Merged
merged 2 commits into from
Jan 16, 2023

Conversation

edwardalee
Copy link
Contributor

This PR removes the distinction between _lf_critical_section_enter() and lf_critical_section_enter(), using only the latter, and makes the function always available, not just for unthreaded targets. Similarly for lf_critical_section_exit() and lf_notify_of_event(). This enables programmers to use these functions. For example, in the MQTT reactors, this is useful when trying to deterministically assign a timestamp to an incoming message.

Copy link
Collaborator

@erlingrj erlingrj left a comment

Choose a reason for hiding this comment

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

Thanks for addressing that. This looks better. I think it should work. The extra level of indirection was added to address the scenario where we have threading:false but tracing:true. In that scenario we want the C-runtime to use normal unthreaded API (e.g. disable interrupts) while the tracing files need the threaded API.

@edwardalee
Copy link
Contributor Author

edwardalee commented Jan 16, 2023

Thanks for addressing that. This looks better. I think it should work. The extra level of indirection was added to address the scenario where we have threading:false but tracing:true. In that scenario we want the C-runtime to use normal unthreaded API (e.g. disable interrupts) while the tracing files need the threaded API.

(Oops, hit the wrong button) If we later need a mix of unthreaded with tracing, I suggest we edit the tracing code to use the critical section mechanism.

@edwardalee edwardalee closed this Jan 16, 2023
@edwardalee edwardalee reopened this Jan 16, 2023
@erlingrj
Copy link
Collaborator

Currently there is a mix of threaded and unthreaded in that case. It will work with linux/mac/win since the platform support implementing threaded/unthreaded is header-only. So trace.c can include the threaded version and reactor.c can include the unthreaded. If you look in trace.c you will see that it defines _LF_TRACE before including platform.h and then undefs it. This is to achieve this. The reason it is needed is that tracing creates threads and uses mutexes. We had some discussion of whether tracing could be implemented without any thread-support.

@edwardalee
Copy link
Contributor Author

The test failure has nothing to do with this PR and has been showing up intermittently. I think it is due to excessively large debug output and I've separately pushed a change that should reduce the size. Merging this PR.

@edwardalee edwardalee merged commit 3fd9f4d into main Jan 16, 2023
@edwardalee edwardalee deleted the critical-section branch January 16, 2023 11:13
@lhstrh lhstrh changed the title Make critical section functions part of the public API Critical section functions as part of the public API Jan 26, 2023
@lhstrh lhstrh added the feature New feature label Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants