Hi Danny, The entry should said something like: - prevent abandoning critical section lock when calling close() on an invalid file descriptor.
Let me know if you rather I submit the change log as part of the patch. So, should we research on how IO operations should behave in MT world? :) On Fri, Aug 29, 2008 at 10:34 PM, Danny Backx <[EMAIL PROTECTED]> wrote: > The patch looks good. Your arguments are valid too. I'd take approach #2 > if I were implementing newlib, but your argument #3 "why do we care" is > certainly valid. > > The only thing I need to commit this is a ChangeLog entry. > > Danny > > On Fri, 2008-08-29 at 13:34 -0700, Pawel Veselov wrote: >> This attached patch should take care of the _close_r() problem. As you >> suggested, I extended FD_CHECK to take 2 arguments. >> >> Regarding protecthing the _fdtab[] contents... Locking the section >> completely during read/write operations will serialize IO of any MT >> program. As it now happens with _write_r(), only one thread can write >> to any file at a time. That doesn't sound right. >> >> I see three potential ways out of this: >> >> 1. In the beginning of a function, lock the section, copy the contents >> of _fdtab[fd] to a local variable, and release the lock. It doesn't >> look like any IO involved methods change the contents of the fd >> structure, but I haven't really looked at what happens to the "cxt", >> and whether we need to worry about applications that try to access the >> same fd simultaneously from multiple threads. There seems to be no >> POSIX statements about MT safety of read()/write() system calls. >> >> 2. add critical section structure as a member of _fdent_s structure. >> Here, I'm not sure how big the structure actually is in memory, and >> whether it is going present significant memory consumption, and >> whether there are any other vital resources that are allocated for the >> initialized critical section, but MSDN says that >> InitializeCriticalSection may return OUT_OF_MEMORY, so there must be >> some other resources that are being allocated, and this may not turn >> out to be feasible. >> >> 3. Why do we care about locking? :) It seems that if an application is >> careless enough to execute IO operations on the same file descriptor >> simultaneously from multiple threads, it's an application issue, and >> it should lead to unpredictable results. The library should only care >> about not corrupting it's structures to the point where accessing them >> will lead to a system exception. >> >> Thanks, >> Pawel. >> >> P.S. The source file seems to be in DOS format, and there is no >> svn:eol-style property set... Shouldn't it be? The patch therefore is >> half in DOS format :) (generated using svn diff) >> >> On Fri, Aug 29, 2008 at 11:04 AM, Danny Backx <[EMAIL PROTECTED]> wrote: >> > On Thu, 2008-08-28 at 19:29 -0700, Pawel Veselov wrote: >> >> Hi, >> >> >> >> Was looking at the _close_r() in >> >> src/newlib/newlib/libc/sys/wince/io.c, at the top: >> >> >> >> EnterCriticalSection(&critsect); >> >> FDCHECK(fd); >> >> >> >> FDCHECK() macro can return from the method if file handle is invalid, >> >> and if it does, wouldn't that leave the lock, and cause deadlock for I/O >> >> operations? >> > >> > That's a very sharp observation ! Thanks for pointing this out. >> > >> >> P.S. What does critsect protect there? Changes to fd[*].fd ? >> > >> > I'd say all of the _fdtab array. However, as you point out, in some >> > places it's not coded very well. More browsing through that source make >> > wonder about the quality of that code. The _read_r() function uses this >> > same _fdtab[] but never uses the critical section. >> > >> > Would you be willing to submit a fix for this ? >> > >> > My first idea would be to add a second argument to FDCHECK which could >> > be the statement to run before the return. >> > >> > Danny >> > -- >> > Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info >> > >> > >> >> >> >> ------------------------------------------------------------------------- >> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge >> Build the coolest Linux based applications with Moblin SDK & win great prizes >> Grand prize is a trip for two to an Open Source event anywhere in the world >> http://moblin-contest.org/redirect.php?banner_id=100&url=/ >> _______________________________________________ Cegcc-devel mailing list >> Cegcc-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/cegcc-devel > -- > Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info > > -- With best of best regards Pawel S. Veselov ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ Cegcc-devel mailing list Cegcc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/cegcc-devel
