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

tests: some tests don't work with newlib lock functions. #12732

Open
gschorcht opened this issue Nov 17, 2019 · 2 comments
Open

tests: some tests don't work with newlib lock functions. #12732

gschorcht opened this issue Nov 17, 2019 · 2 comments
Assignees
Labels
Area: tests Area: tests and testing framework Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Comments

@gschorcht
Copy link
Contributor

gschorcht commented Nov 17, 2019

Description

The following test applications call puts inside an ISR which can lead to problems when newlib's locking functions are implemented by the platform:

tests/isr_yield_higher
tests/periph_rtc

Background

newlib's reentrant versions of system calls use lock functions for different lock objects. For example, puts locks the stdout file using _lock_acquire_recursive. While the lock functions in single-thread environments are only dummies, these functions would have to be implemented by the operating system in multi-thread environments.

The only reasonable way in RIOT to realize these lock functions

_lock_acquire
_lock_acquire_reusrive
_lock_try_acquire
_lock_try_acquire_reusrive

seems to be RIOT's built-in mechanism mutex and rmutex. Since mutex and rmutex don't work in interrupt context, system calls must not be used in ISRs.

The problem was found out in PR #11108.

Steps to reproduce

Enable locking functions in cpu/esp8266/syscalls.c in PR #11108 and flash tests/isr_yield_higher

make BOARD=esp8266-esp-12x -C tests/isr_yield_higher
@gschorcht gschorcht added Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: tests Area: tests and testing framework labels Dec 9, 2019
@gschorcht gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) and removed Area: core Area: RIOT kernel. Handle PRs marked with this with care! labels Dec 9, 2019
@gschorcht
Copy link
Contributor Author

The question is, how we should deal with it. It is more a general problem.

@gschorcht gschorcht changed the title tests/isr_yield_higher: doesn't work with newlib lock functions. tests: some tests don't work with newlib lock functions. Dec 28, 2019
@gschorcht
Copy link
Contributor Author

@kaspar030 For the moment, I could find a hack which solves the problem with the puts call in the ISR of tests/isr_yield_higher

/*
* disable the locking for stdout/stderr to avoid rmutex based locking
* when puts/printf are called from an ISR
*/
__fsetlocking(_GLOBAL_REENT->_stdout, FSETLOCKING_BYCALLER);
__fsetlocking(_GLOBAL_REENT->_stderr, FSETLOCKING_BYCALLER);

It also solves the problem that assert(!irq_is_in()) in locking functions doesn't work.

assert(!irq_is_in());

I'm not really happy with this hack since it breaks the thread safety for file I/O operations on stdout/stderr. But, at least it's no worse than on other platforms that use newlib_nano, which isn't thread safe at all.

Do you think, this hack is an acceptable way or do we need a more general solution?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tests Area: tests and testing framework Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

No branches or pull requests

4 participants