[ECOS] probable bug in io/serial/common/serial.c!
Mohammed Illyas Mansoor
mansoor@cdotb.ernet.in
Tue Jan 4 22:17:00 GMT 2000
Hi,
I was going through the common serial io code it seems to me
that there probably is a bug in it, below is a snap shot of the code
in question.
<snap>
static Cyg_ErrNo
serial_write(cyg_io_handle_t handle, const void *_buf, cyg_uint32 *len)
{
cyg_devtab_entry_t *t = (cyg_devtab_entry_t *)handle;
serial_channel *chan = (serial_channel *)t->priv;
serial_funs *funs = chan->funs;
cyg_int32 size = *len;
cyg_uint8 *buf = (cyg_uint8 *)_buf;
int next;
cbuf_t *cbuf = &chan->out_cbuf;
Cyg_ErrNo res = ENOERR;
cbuf->abort = false;
cyg_drv_mutex_lock(&cbuf->lock);
if (cbuf->len == 0) {
// Non interrupt driven (i.e. polled) operation
while (size-- > 0) {
while ((funs->putc)(chan, *buf) == false) ; // Ignore full,
keep trying
buf++;
}
} else {
cyg_drv_dsr_lock(); // Avoid race condition testing pointers
while (size > 0) {
next = cbuf->put + 1;
if (next == cbuf->len) next = 0;
if (next == cbuf->get) {
cbuf->waiting = true;
// Buffer full - wait for space
(funs->start_xmit)(chan); // Make sure xmit is running
cbuf->pending += size; // Have this much more to send
eventually]
***---->cyg_drv_cond_wait(&cbuf->wait);<-----***
cbuf->pending -= size;
if (cbuf->abort) {
// Give up!
cbuf->abort = false;
cbuf->waiting = false;
res = -EINTR;
break;
}
} else {
cbuf->data[cbuf->put++] = *buf++;
cbuf->put = next;
size--; // Only count if actually sent!
}
}
cyg_drv_dsr_unlock();
(funs->start_xmit)(chan); // Start output as necessary
}
cyg_drv_mutex_unlock(&cbuf->lock);
return res;
}
</snap>
In the line with ***--->cyg_drv_cond_wait(&cond_wait);<----***,
will cause a deadlock, because before going for a wait on the condition
variable
start_xmt function is called which will trigger the transmitter to send
the char's, if the call to xmt_char finds out that chan->waiting is true
and wakes up the writer thread which was supposed to be waiting by now,
he would also change the chan->waiting to false. Later, when the wait by
the writer is called there will be no waking up of the writer by the
driver
(as it has been already done by the xmt_char() ), causing it to
deadlock.
The fix might be as below,
if (next == cbuf->get) {
cbuf->waiting = true;
// Buffer full - wait for space
(funs->start_xmit)(chan); // Make sure xmit is running
cbuf->pending += size; // Have this much more to send
[eventually]
++if ( cbuf->waiting == true)
cyg_drv_cond_wait(&cbuf->wait);
--With Regards,
M. I. Mansoor.
More information about the Ecos-discuss
mailing list