Piotr Caban : msvcrt: Avoid using global critical section while allocating new file descriptors.
Alexandre Julliard
julliard at wine.codeweavers.com
Thu Jun 11 09:46:11 CDT 2015
Module: wine
Branch: master
Commit: 6ed69c107f82d127f6e87f654fbfcd023498ec52
URL: http://source.winehq.org/git/wine.git/?a=commit;h=6ed69c107f82d127f6e87f654fbfcd023498ec52
Author: Piotr Caban <piotr at codeweavers.com>
Date: Wed Jun 10 18:12:53 2015 +0200
msvcrt: Avoid using global critical section while allocating new file descriptors.
---
dlls/msvcrt/file.c | 146 +++++++++++++++++------------------------------------
1 file changed, 47 insertions(+), 99 deletions(-)
diff --git a/dlls/msvcrt/file.c b/dlls/msvcrt/file.c
index 36887fc..802834c 100644
--- a/dlls/msvcrt/file.c
+++ b/dlls/msvcrt/file.c
@@ -114,9 +114,6 @@ ioinfo * MSVCRT___pioinfo[MSVCRT_MAX_FILES/MSVCRT_FD_BLOCK_SIZE] = { 0 };
*/
ioinfo MSVCRT___badioinfo = { INVALID_HANDLE_VALUE, WX_TEXT };
-static int MSVCRT_fdstart = 3; /* first unallocated fd */
-static int MSVCRT_fdend = 3; /* highest allocated fd */
-
typedef struct {
MSVCRT_FILE file;
CRITICAL_SECTION crit;
@@ -143,12 +140,9 @@ static const ULONGLONG WCBAT = TOUL('b') << 32 | TOUL('a') << 16 | TOUL('t');
static const ULONGLONG WCCMD = TOUL('c') << 32 | TOUL('m') << 16 | TOUL('d');
static const ULONGLONG WCCOM = TOUL('c') << 32 | TOUL('o') << 16 | TOUL('m');
-/* This critical section protects the tables MSVCRT___pioinfo and MSVCRT_fstreams,
- * and their related indexes, MSVCRT_fdstart, MSVCRT_fdend,
- * and MSVCRT_stream_idx, from race conditions.
- * It doesn't protect against race conditions manipulating the underlying files
- * or flags; doing so would probably be better accomplished with per-file
- * protection, rather than locking the whole table for every change.
+/* This critical section protects the MSVCRT_fstreams table
+ * and MSVCRT_stream_idx from race conditions. It also
+ * protects fd critical sections creation code.
*/
static CRITICAL_SECTION MSVCRT_file_cs;
static CRITICAL_SECTION_DEBUG MSVCRT_file_cs_debug =
@@ -257,19 +251,24 @@ static inline ioinfo* get_ioinfo_nolock(int fd)
return ret + (fd%MSVCRT_FD_BLOCK_SIZE);
}
-static inline ioinfo* get_ioinfo(int fd)
+static inline void init_ioinfo_cs(ioinfo *info)
{
- ioinfo *ret = get_ioinfo_nolock(fd);
- if(ret == &MSVCRT___badioinfo)
- return ret;
- if(!(ret->exflag & EF_CRIT_INIT)) {
+ if(!(info->exflag & EF_CRIT_INIT)) {
LOCK_FILES();
- if(!(ret->exflag & EF_CRIT_INIT)) {
- InitializeCriticalSection(&ret->crit);
- ret->exflag |= EF_CRIT_INIT;
+ if(!(info->exflag & EF_CRIT_INIT)) {
+ InitializeCriticalSection(&info->crit);
+ info->exflag |= EF_CRIT_INIT;
}
UNLOCK_FILES();
}
+}
+
+static inline ioinfo* get_ioinfo(int fd)
+{
+ ioinfo *ret = get_ioinfo_nolock(fd);
+ if(ret == &MSVCRT___badioinfo)
+ return ret;
+ init_ioinfo_cs(ret);
EnterCriticalSection(&ret->crit);
return ret;
}
@@ -301,77 +300,49 @@ static inline BOOL alloc_pioinfo_block(int fd)
static inline ioinfo* get_ioinfo_alloc_fd(int fd)
{
- ioinfo *ret, *info;
+ ioinfo *ret;
ret = get_ioinfo(fd);
if(ret != &MSVCRT___badioinfo)
- {
- LOCK_FILES();
- /* locate next free slot */
- if (fd == MSVCRT_fdstart && fd == MSVCRT_fdend)
- MSVCRT_fdstart = MSVCRT_fdend + 1;
- else if (fd == MSVCRT_fdstart)
- {
- MSVCRT_fdstart++;
- while (MSVCRT_fdstart < MSVCRT_fdend &&
- ((info = get_ioinfo_nolock(MSVCRT_fdstart))->exflag & EF_CRIT_INIT))
- {
- if (TryEnterCriticalSection(&info->crit))
- {
- if (info->handle == INVALID_HANDLE_VALUE)
- {
- LeaveCriticalSection(&info->crit);
- break;
- }
- LeaveCriticalSection(&info->crit);
- }
- MSVCRT_fdstart++;
- }
- }
- /* update last fd in use */
- if (fd >= MSVCRT_fdend)
- MSVCRT_fdend = fd + 1;
- TRACE("fdstart is %d, fdend is %d\n", MSVCRT_fdstart, MSVCRT_fdend);
- UNLOCK_FILES();
return ret;
- }
if(!alloc_pioinfo_block(fd))
return &MSVCRT___badioinfo;
- ret = get_ioinfo(fd);
- if(ret == &MSVCRT___badioinfo)
- return ret;
+ return get_ioinfo(fd);
+}
- LOCK_FILES();
- /* locate next free slot */
- if (fd == MSVCRT_fdstart && fd == MSVCRT_fdend)
- MSVCRT_fdstart = MSVCRT_fdend + 1;
- else if (fd == MSVCRT_fdstart)
+static inline ioinfo* get_ioinfo_alloc(int *fd)
+{
+ int i;
+
+ *fd = -1;
+ for(i=0; i<MSVCRT_MAX_FILES; i++)
{
- MSVCRT_fdstart++;
- while (MSVCRT_fdstart < MSVCRT_fdend &&
- ((info = get_ioinfo_nolock(MSVCRT_fdstart))->exflag & EF_CRIT_INIT))
+ ioinfo *info = get_ioinfo_nolock(i);
+
+ if(info == &MSVCRT___badioinfo)
+ {
+ if(!alloc_pioinfo_block(i))
+ return &MSVCRT___badioinfo;
+ info = get_ioinfo_nolock(i);
+ }
+
+ init_ioinfo_cs(info);
+ if(TryEnterCriticalSection(&info->crit))
{
- if (TryEnterCriticalSection(&info->crit))
+ if(info->handle == INVALID_HANDLE_VALUE)
{
- if (info->handle == INVALID_HANDLE_VALUE)
- {
- LeaveCriticalSection(&info->crit);
- break;
- }
- LeaveCriticalSection(&info->crit);
+ *fd = i;
+ return info;
}
- MSVCRT_fdstart++;
+ LeaveCriticalSection(&info->crit);
}
}
- /* update last fd in use */
- if (fd >= MSVCRT_fdend)
- MSVCRT_fdend = fd + 1;
- TRACE("fdstart is %d, fdend is %d\n", MSVCRT_fdstart, MSVCRT_fdend);
- UNLOCK_FILES();
- return ret;
+ WARN(":files exhausted!\n");
+ *MSVCRT__errno() = MSVCRT_ENFILE;
+ return &MSVCRT___badioinfo;
}
static inline void release_ioinfo(ioinfo *info)
@@ -434,13 +405,6 @@ static void msvcrt_free_fd(int fd)
}
}
release_ioinfo(fdinfo);
-
- LOCK_FILES();
- if (fd == MSVCRT_fdend - 1)
- MSVCRT_fdend--;
- if (fd < MSVCRT_fdstart)
- MSVCRT_fdstart = fd;
- UNLOCK_FILES();
}
static void msvcrt_set_fd(ioinfo *fdinfo, HANDLE hand, int flag)
@@ -463,23 +427,11 @@ static void msvcrt_set_fd(ioinfo *fdinfo, HANDLE hand, int flag)
/* INTERNAL: Allocate an fd slot from a Win32 HANDLE */
static int msvcrt_alloc_fd(HANDLE hand, int flag)
{
- ioinfo *info;
int fd;
+ ioinfo *info = get_ioinfo_alloc(&fd);
- LOCK_FILES();
- fd = MSVCRT_fdstart;
TRACE(":handle (%p) allocating fd (%d)\n", hand, fd);
- if (fd >= MSVCRT_MAX_FILES)
- {
- UNLOCK_FILES();
- WARN(":files exhausted!\n");
- *MSVCRT__errno() = MSVCRT_ENFILE;
- return -1;
- }
-
- info = get_ioinfo_alloc_fd(fd);
- UNLOCK_FILES();
if(info == &MSVCRT___badioinfo)
return -1;
@@ -623,9 +575,6 @@ void msvcrt_init_io(void)
wxflag_ptr++; handle_ptr++;
}
- MSVCRT_fdend = max( 3, count );
- for (MSVCRT_fdstart = 3; MSVCRT_fdstart < MSVCRT_fdend; MSVCRT_fdstart++)
- if (get_ioinfo_nolock(MSVCRT_fdstart)->handle == INVALID_HANDLE_VALUE) break;
}
fdinfo = get_ioinfo_alloc_fd(MSVCRT_STDIN_FILENO);
@@ -1147,14 +1096,13 @@ int CDECL MSVCRT__dup2(int od, int nd)
int CDECL MSVCRT__dup(int od)
{
int fd, ret;
+ ioinfo *info = get_ioinfo_alloc(&fd);
- LOCK_FILES();
- fd = MSVCRT_fdstart;
if (MSVCRT__dup2(od, fd) == 0)
ret = fd;
else
ret = -1;
- UNLOCK_FILES();
+ release_ioinfo(info);
return ret;
}
@@ -4377,7 +4325,7 @@ MSVCRT_FILE* CDECL MSVCRT__wfreopen(const MSVCRT_wchar_t *path, const MSVCRT_wch
TRACE(":path (%s) mode (%s) file (%p) fd (%d)\n", debugstr_w(path), debugstr_w(mode), file, file ? file->_file : -1);
LOCK_FILES();
- if (!file || ((fd = file->_file) < 0) || fd > MSVCRT_fdend)
+ if (!file || ((fd = file->_file) < 0))
file = NULL;
else
{
More information about the wine-cvs
mailing list