Mon 22 Sep 2014 02:18:05 PM UTC, original submission:
In event_callback(), the for loop at the bottom that handles signaling waiting application threads that a select is ready, operates under the assumption that the call to sys_sem_signal() won't cause a context switch. The entire loop is protected by a pair of SYS_ARCH_PROTECT() and SYS_ARCH_UNPROTECT().
On certain ports, the _PROTECT and _UNPROTECT calls just disable/enable interrupts. This leaves the possibility of a context switch happening in the call to sys_sem_signal() if the OS primitive preempts the LwIP thread with the waiting task.
The for loop already has support for handling a context switch each iteration and saves the rolling sequence counter in order to detect a context switch when SYS_ARCH_UNPROTECT() is called.
We can address the potential context switch during the call to sys_sem_signal() by moving the line of last_select_cb_ctr = select_ct_ctr to the top of the for loop body, thus ensure any context switch within the loop body is detected. Since we don't modify the select list within the loop body, this is safe
SYS_ARCH_PROTECT(lev);
...
again:
for (scb = select_cb_list; scb != NULL; scb = scb->next) {
if (scb->sem_signalled == 0) {
/* semaphore not signalled yet */
int do_signal = 0;
/* Test this select call for our socket */
if (sock->rcvevent > 0) {
if (scb->readset && FD_ISSET(s, scb->readset)) {
do_signal = 1;
}
}
if (sock->sendevent != 0) {
if (!do_signal && scb->writeset && FD_ISSET(s, scb->writeset)) {
do_signal = 1;
}
}
if (sock->errevent != 0) {
if (!do_signal && scb->exceptset && FD_ISSET(s, scb->exceptset)) {
do_signal = 1;
}
}
if (do_signal) {
scb->sem_signalled = 1;
/* Don't call SYS_ARCH_UNPROTECT() before signaling the semaphore, as this might
lead to the select thread taking itself off the list, invalidagin the semaphore. */
sys_sem_signal(&scb->sem);
}
}
last_select_cb_ctr = select_cb_ctr;
/* unlock interrupts with each step */
SYS_ARCH_UNPROTECT(lev);
/* this makes sure interrupt protection time is short */
SYS_ARCH_PROTECT(lev);
if (last_select_cb_ctr != select_cb_ctr) {
/* someone has changed select_cb_list, restart at the beginning */
goto again;
}
|