This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
RE: Race conditions in serial_select
- From: "Gaer, A." <Andreas dot Gaer at baslerweb dot com>
- To: "'Jay Foster'" <jay at systech dot com>, "'Andrew Lunn'" <andrew at lunn dot ch>
- Cc: ecos-patches at ecos dot sourceware dot org
- Date: Tue, 21 Jun 2005 17:01:29 +0200
- Subject: RE: Race conditions in serial_select
Hello!
As the DSR IMHO shouldn't try to accquire the mutex lock anyway, I would
guess that it is save to do the locking in the proposed way. But I must
admit that I intended to do it exactly the other way around like it was done
in all the other functions of the serial.c file, simply to keep the logic a
bit consistent. Don't ask me why I got it wrong, I must have been completely
blind that afternoon. Sorry.
I can resend the patch with the locks swapped, if this is wanted.
As I understand it, the mutex lock is used to synchronize access to the
read/write buffers from concurrent threads. As the logic of the select
depends on the consitency of the buffers during it's exectution, I would say
the mutex lock is neccessary. Though I can't imagine any usecase where the
serial select and read/write calls are placed into different threads - but
who knows.
Best regards,
Andreas.
> -----Original Message-----
> From: Jay Foster [mailto:jay@systech.com]
> Sent: Tuesday, June 21, 2005 12:00 AM
> To: 'Andrew Lunn'; Gaer, A.
> Cc: ecos-patches@ecos.sourceware.org
> Subject: RE: Race conditions in serial_select
>
>
> I took a look at this change for considering whether to apply
> it to my code
> base. It looks like there may now be a deadlock problem with
> the new code.
> In the serial.c file, the code obtains and releases the locks
> as follows:
>
> cyg_drv_mutex_lock(...);
> ...
> cyg_drv_dsr_lock(...);
>
> ...
>
> cyg_drv_dsr_unlock(...);
> ...
> cyg_drv_mutex_unlock(...);
>
> With the change in serial_select(), the order is reversed
> (i.e. the DSR lock
> is aquired before the mutex lock). This can lead to a
> deadlock condition.
> Would not just the DSR lock be sufficient?
>
> Jay Foster
>
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Monday, June 20, 2005 11:15 AM
> To: Gaer, A.
> Cc: ecos-patches@ecos.sourceware.org
> Subject: Re: Race conditions in serial_select
>
>
> On Fri, Jun 17, 2005 at 11:47:09AM +0200, Gaer, A. wrote:
> > Hello all!
> >
> > The serial_select() implementation from
> > "io/serial/current/src/common/serial.c" doesn't care about
> concurrent
> access
> > to the cbuf structure. This leads to race conditions when
> the serial DSR
> > routine changes the counters in this structure while select
> checks for
> > "cbuf->nb == 0". As a result, user threads that sleep
> because of a select
> > call will not be woken up, even if data arrives. Also access to this
> > structure isn't implemented thread-safe because of missing
> mutex locks.
> The
> > attached patch fixes this problems.
>
> Thanks
> Andrew
>