time for 1.5.11?
Igor Pechtchanski
pechtcha@cs.nyu.edu
Fri Jun 11 00:31:00 GMT 2004
On Wed, 9 Jun 2004, Igor Pechtchanski wrote:
> On Wed, 9 Jun 2004, Christopher Faylor wrote:
>
> > On Wed, Jun 09, 2004 at 04:28:37PM -0400, Igor Pechtchanski wrote:
> > >On Wed, 9 Jun 2004, Igor Pechtchanski wrote:
> > >
> > >> On Wed, 9 Jun 2004, Corinna Vinschen wrote:
> > >>
> > >> > On Jun 9 13:01, Christopher Faylor wrote:
> > >> > > Anyway, on to the future... Would it make sense to release a 1.5.11?
> > >> > > Do you think we've hit all of the fallout from the path reorg/speedup?
> > >> >
> > >> > Looking through the reports on the Cygwin list, I see that we never
> > >> > got a reply if the MapViewOfFileEx problem has been solved. Also,
> > >> > what about the report that /dev/dsp isn't working? Igor was going to
> > >> > investigate this further. Any news, Igor?
> > >>
> > >> I'm still investigating. So far it looks like /dev/dsp doesn't like to be
> > >> dup'd (but then, who does? ;-)). I'm attaching a short test program to
> > >> demostrate this, but I'll keep digging...
> > >
> > >...And moments after I sent the above I found the culprit (I think).
> > >Seems like fhandler_dsp::dup() neglects to call fhandler_base::dup(), so
> > >the IO handle is unset. It should be a trivial 1-line patch, but just in
> > >case, can someone with a copyright assignment take care of this?
> >
> > You should be safe if it is really a one line patch. Don't you want to
> > get your name into the Cygwin changelog limelight?
> >
> > cgf
>
> Ok, it turns out that to fix this properly, we'll need a much larger patch
> than the one line I was looking at... Here are some thoughts.
>
> First off, my guess above was wrong. The IO handle has nothing to do with
> this -- fhandler_dev_dsp doesn't use it. However, the intuition (I still
> think) was correct: some fields, namely the audio_in_ and audio_out_
> pointers, weren't replicated on dup(), so the new (duped) object ended up
> with NULL fields, and couldn't write to them.
>
> The simplest solution to the above is to simply copy the pointers. This
> will work for the test program I provided earlier, but won't for another
> legitimate test program (attached) that closes the original handle before
> using the duped one (I think this behavior is allowed).
>
> The "proper" way to fix this, IMO, would be to allow sharing the audio_in_
> and audio_out_ objects, and have some sort of reference count in them,
> instead of deleting them outright in close(). I'm not sure this will
> cover the situations when a process is spawned (e.g., fork), but it should
> suffice for redirection. This, however, promises to be a non-trivial
> patch.
>
> Comments?
> Igor
Ok, some more data here. I have a reference count patch ready (the
assignment's in the mail). However, there's some weird data race going on
with it. If I insert a "sleep(2)" just before each "close(dst)" in the
attached testcase, it works. But if I attempt to close the /dev/dsp file
without the sleep (before the data has finished going out to the card?), I
get memory and/or stack corruption (SEGVs or 100% CPU due to huge bogus
lengths in memcpy). I have trouble catching it, since without the delay
the stack becomes corrupted, and a step-by-step execution causes a delay,
and thus succeeds.
Given that it works with the sleep(), I think I'm going to send the patch
for review anyway (later), but I'd like to track this down -- there's
something there that's thread-unsafe, apparently, and looks like my
changes either caused or exposed this.
Igor
--
http://cs.nyu.edu/~pechtcha/
|\ _,,,---,,_ pechtcha@cs.nyu.edu
ZZZzz /,`.-'`' -. ;-;;,_ igor@watson.ibm.com
|,4- ) )-,_. ,\ ( `'-' Igor Pechtchanski, Ph.D.
'---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow!
"I have since come to realize that being between your mentor and his route
to the bathroom is a major career booster." -- Patrick Naughton
-------------- next part --------------
#include <fcntl.h>
#include <unistd.h>
#define BUFSIZE 1024
#define WORKS "This works\n"
#define NWORKS "This doesn\'t work\n"
int main(int ac, char *av[]) {
char buf[BUFSIZE+1];
int num, src, dst;
write(2, WORKS, sizeof(WORKS)-1);
src = open("/cygdrive/c/WINNT/Media/tada.wav", O_RDONLY);
dst = open("/dev/dsp", 0x601);
while ((num = read(src, buf, BUFSIZE)) > 0)
write(dst, buf, num);
close(dst);
close(src);
write(2, NWORKS, sizeof(NWORKS)-1);
src = open("/cygdrive/c/WINNT/Media/tada.wav", O_RDONLY);
dst = open("/dev/dsp", 0x601);
if (dup2(dst, 1) != -1) {
close(dst);
dst = 1;
while ((num = read(src, buf, BUFSIZE)) > 0)
write(dst, buf, num);
}
close(dst);
close(src);
}
More information about the Cygwin-developers
mailing list