This is the mail archive of the cygwin-developers@cygwin.com mailing list for the Cygwin project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
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
Attachment:
dsp_dup_close.c
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |