PlaneteCasio
/
libc
1
0
Fork 0
Code Issues 6 Pull Requests Releases Wiki Activity

* DevNotes: Add entry cgf-000011. 

* fhandler.h (fhandler_base::refcnt): Delete.
(fhandler_base::inc_refcnt): New function.
(fhandler_base::dec_refcnt): New function.
* cygheap.h (cygheap_fdnew::~cygheap_fdnew): Accommodate split of refcnt to
inc_refcnt/dec_refcnt.
(cygheap_fdget::cygheap_fdget): Ditto.
(cygheap_fdget::~cygheap_fdget::cygheap_fdget): Ditto.
* dtable.cc (dtable::release): Ditto.
(cygwin_attach_handle_to_fd): Ditto.
(dtable::init_std_file_from_handle): Ditto.
(dtable::dup3): On success, return with fdtab locked.
* dtable.h (dtable): Add dup_finish as a friend.
* syscalls.cc (dup_finish): Define new function.  Increment refcnt while fdtab
is locked.
(dup2): Use common dup_finish() to perform dup operation.
(dup3): Ditto.
This commit is contained in:
Christopher Faylor 2012-06-03 18:02:45 +00:00
parent ff80d22a7c
commit 3143cb7c00
7 changed files with 74 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
winsup/cygwin/ChangeLog
View File

@ -1,3 +1,23 @@
2012-06-03 Christopher Faylor <me.cygwin2012@cgf.cx>
* DevNotes: Add entry cgf-000011.
* fhandler.h (fhandler_base::refcnt): Delete.
(fhandler_base::inc_refcnt): New function.
(fhandler_base::dec_refcnt): New function.
* cygheap.h (cygheap_fdnew::~cygheap_fdnew): Accommodate split of
refcnt to inc_refcnt/dec_refcnt.
(cygheap_fdget::cygheap_fdget): Ditto.
(cygheap_fdget::~cygheap_fdget::cygheap_fdget): Ditto.
* dtable.cc (dtable::release): Ditto.
(cygwin_attach_handle_to_fd): Ditto.
(dtable::init_std_file_from_handle): Ditto.
(dtable::dup3): On success, return with fdtab locked.
* dtable.h (dtable): Add dup_finish as a friend.
* syscalls.cc (dup_finish): Define new function. Increment refcnt
while fdtab is locked.
(dup2): Use common dup_finish() to perform dup operation.
(dup3): Ditto.
2012-06-03 Corinna Vinschen <corinna@vinschen.de>
* globals.cc (ro_u_refs): New R/O unicode string.

25
winsup/cygwin/DevNotes
View File

@ -1,3 +1,28 @@
2012-06-02 cgf-000011
The refcnt handling was tricky to get right but I had convinced myself
that the refcnt's were always incremented/decremented under a lock.
Corinna's 2012-05-23 change to refcnt exposed a potential problem with
dup handling where the fdtab could be updated while not locked.
That should be fixed by this change but, on closer examination, it seems
ilke there are many places where it is possible for the refcnt to be
updated while the fdtab is not locked since the default for
cygheap_fdget is to not lock the fdtab (and that should be the default -
you can't have read holding a lock).
Since refcnt was only ever called with 1 or -1, I broke it up into two
functions but kept the Interlocked* operation. Incrementing a variable
should not be as racy as adding an arbitrary number to it but we have
InterlockedIncrement/InterlockedDecrement for a reason so I kept the
Interlocked operation here.
In the meantime, I'll be mulling over whether the refcnt operations are
actually safe as they are. Maybe just ensuring that they are atomically
updated is enough since they control the destruction of an fh. If I got
the ordering right with incrementing and decrementing then that should
be adequate.
2012-06-02 cgf-000010
<1.7.16>

6
winsup/cygwin/cygheap.h
View File

@ -458,7 +458,7 @@ class cygheap_fdnew : public cygheap_fdmanip
~cygheap_fdnew ()
{
if (cygheap->fdtab[fd])
cygheap->fdtab[fd]->refcnt (1);
cygheap->fdtab[fd]->inc_refcnt ();
}
void operator = (fhandler_base *fh) {cygheap->fdtab[fd] = fh;}
};
@ -476,7 +476,7 @@ public:
this->fd = fd;
locked = lockit;
fh = cygheap->fdtab[fd];
fh->refcnt (1);
fh->inc_refcnt ();
}
else
{
@ -491,7 +491,7 @@ public:
}
~cygheap_fdget ()
{
if (fh && fh->refcnt (-1) <= 0)
if (fh && fh->dec_refcnt () <= 0)
{
debug_only_printf ("deleting fh %p", fh);
delete fh;

11
winsup/cygwin/dtable.cc
View File

@ -243,7 +243,7 @@ dtable::release (int fd)
{
if (fds[fd]->need_fixup_before ())
dec_need_fixup_before ();
fds[fd]->refcnt (-1);
fds[fd]->dec_refcnt ();
fds[fd] = NULL;
if (fd <= 2)
set_std_handle (fd);
@ -259,7 +259,7 @@ cygwin_attach_handle_to_fd (char *name, int fd, HANDLE handle, mode_t bin,
if (!fh)
return -1;
cygheap->fdtab[fd] = fh;
cygheap->fdtab[fd]->refcnt (1);
cygheap->fdtab[fd]->inc_refcnt ();
fh->init (handle, myaccess, bin ?: fh->pc_binmode ());
return fd;
}
@ -398,7 +398,7 @@ dtable::init_std_file_from_handle (int fd, HANDLE handle)
fh->open_setup (openflags);
fh->usecount = 0;
cygheap->fdtab[fd] = fh;
cygheap->fdtab[fd]->refcnt (1);
cygheap->fdtab[fd]->inc_refcnt ();
set_std_handle (fd);
paranoid_printf ("fd %d, handle %p", fd, handle);
}
@ -713,6 +713,7 @@ dtable::dup3 (int oldfd, int newfd, int flags)
MALLOC_CHECK;
debug_printf ("dup3 (%d, %d, %p)", oldfd, newfd, flags);
lock ();
bool do_unlock = true;
if (not_open (oldfd))
{
@ -760,6 +761,7 @@ dtable::dup3 (int oldfd, int newfd, int flags)
goto done;
}
do_unlock = false;
fds[newfd] = newfh;
if ((res = newfd) <= 2)
@ -767,7 +769,8 @@ dtable::dup3 (int oldfd, int newfd, int flags)
done:
MALLOC_CHECK;
unlock ();
if (do_unlock)
unlock ();
syscall_printf ("%R = dup3(%d, %d, %p)", res, oldfd, newfd, flags);
return res;

1
winsup/cygwin/dtable.h
View File

@ -89,6 +89,7 @@ public:
void fixup_before_fork (DWORD win_proc_id);
friend void dtable_init ();
friend void __stdcall close_all_files (bool);
friend int dup_finish (int, int, int);
friend class fhandler_disk_file;
friend class cygheap_fdmanip;
friend class cygheap_fdget;

11
winsup/cygwin/fhandler.h
View File

@ -182,15 +182,8 @@ class fhandler_base
HANDLE read_state;
public:
long refcnt(long i = 0)
{
debug_only_printf ("%p, %s, i %d, refcnt %ld", this, get_name (), i, _refcnt + i);
/* This MUST be an interlocked operation. If multiple threads access the
same descriptor in quick succession, a context switch can interrupt
the += operation and we wrongly end up with a refcnt of 0 in the
cygheap_fdget destructor. */
return i ? InterlockedAdd (&_refcnt, i) : _refcnt;
}
long inc_refcnt () {return InterlockedIncrement (&_refcnt);}
long dec_refcnt () {return InterlockedDecrement (&_refcnt);}
class fhandler_base *archetype;
int usecount;

20
winsup/cygwin/syscalls.cc
View File

@ -126,6 +126,18 @@ dup (int fd)
return res;
}
inline int
dup_finish (int oldfd, int newfd, int flags)
{
int res;
if ((res = cygheap->fdtab.dup3 (oldfd, newfd, flags)) == newfd)
{
cygheap_fdget (newfd)->inc_refcnt ();
cygheap->fdtab.unlock (); /* dup3 exits with lock set on success */
}
return res;
}
extern "C" int
dup2 (int oldfd, int newfd)
{
@ -140,8 +152,8 @@ dup2 (int oldfd, int newfd)
cygheap_fdget cfd (oldfd);
res = (cfd >= 0) ? oldfd : -1;
}
else if ((res = cygheap->fdtab.dup3 (oldfd, newfd, 0)) == newfd)
cygheap->fdtab[newfd]->refcnt (1);
else
res = dup_finish (oldfd, newfd, 0);
syscall_printf ("%R = dup2(%d, %d)", res, oldfd, newfd);
return res;
@ -162,8 +174,8 @@ dup3 (int oldfd, int newfd, int flags)
set_errno (cfd < 0 ? EBADF : EINVAL);
res = -1;
}
else if ((res = cygheap->fdtab.dup3 (oldfd, newfd, flags)) == newfd)
cygheap->fdtab[newfd]->refcnt (1);
else
res = dup_finish (oldfd, newfd, flags);
syscall_printf ("%R = dup3(%d, %d, %p)", res, oldfd, newfd, flags);
return res;