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.
      |\      _,,,---,,_		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);

  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) {
    dst = 1;
    while ((num = read(src, buf, BUFSIZE)) > 0)
      write(dst, buf, num);

More information about the Cygwin-developers mailing list