windows: improved handling of invalid FDs

If passed and invalid FD, _get_osfhandle() sets an error code
through errno, not _doserrno. Hence we need to use
SET_REQ_WIN32_ERROR insted of SET_REQ_RESULT.

In debug builds, _get_osfhandle() also raises a superfluous
assert. I implemented a wrapper that disables all asserts
around the call to _get_osfhandle().

This fixes node.js unit tests test-fs-read-stream.js and
test-listen-fd-ebadf.js.
This commit is contained in:
Alexis Campailla 2013-12-17 04:26:28 -08:00 committed by Bert Belder
parent 4ce9c39dd8
commit c0716b3d9f
8 changed files with 84 additions and 13 deletions

View File

@ -26,6 +26,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <crtdbg.h>
#include "uv.h"
#include "internal.h"
@ -41,6 +42,32 @@ static uv_once_t uv_init_guard_ = UV_ONCE_INIT;
static uv_once_t uv_default_loop_init_guard_ = UV_ONCE_INIT;
#ifdef _DEBUG
/* Our crt debug report handler allows us to temporarily disable asserts */
/* just for the current thread. */
__declspec( thread ) int uv__crt_assert_enabled = TRUE;
static int uv__crt_dbg_report_handler(int report_type, char *message, int *ret_val) {
if (uv__crt_assert_enabled || report_type != _CRT_ASSERT)
return FALSE;
if (ret_val) {
/* Set ret_val to 0 to continue with normal execution. */
/* Set ret_val to 1 to trigger a breakpoint. */
if(IsDebuggerPresent())
*ret_val = 1;
else
*ret_val = 0;
}
/* Don't call _CrtDbgReport. */
return TRUE;
}
#endif
static void uv__crt_invalid_parameter_handler(const wchar_t* expression,
const wchar_t* function, const wchar_t * file, unsigned int line,
uintptr_t reserved) {
@ -59,6 +86,13 @@ static void uv_init(void) {
_set_invalid_parameter_handler(uv__crt_invalid_parameter_handler);
#endif
/* We also need to setup our debug report handler because some CRT */
/* functions (eg _get_osfhandle) raise an assert when called with invalid */
/* FDs even though they return the proper error code in the release build. */
#ifdef _DEBUG
_CrtSetReportHook(uv__crt_dbg_report_handler);
#endif
/* Fetch winapi function pointers. This must be done first because other */
/* intialization code might need these function pointers to be loaded. */
uv_winapi_init();

View File

@ -34,6 +34,7 @@
#include "uv.h"
#include "internal.h"
#include "req-inl.h"
#include "handle-inl.h"
#define UV_FS_FREE_PATHS 0x0002
@ -548,9 +549,10 @@ void fs__read(uv_fs_t* req) {
VERIFY_FD(fd, req);
handle = (HANDLE) _get_osfhandle(fd);
handle = uv__get_osfhandle(fd);
if (handle == INVALID_HANDLE_VALUE) {
SET_REQ_RESULT(req, -1);
SET_REQ_WIN32_ERROR(req, ERROR_INVALID_HANDLE);
return;
}
@ -595,9 +597,9 @@ void fs__write(uv_fs_t* req) {
VERIFY_FD(fd, req);
handle = (HANDLE) _get_osfhandle(fd);
handle = uv__get_osfhandle(fd);
if (handle == INVALID_HANDLE_VALUE) {
SET_REQ_RESULT(req, -1);
SET_REQ_WIN32_ERROR(req, ERROR_INVALID_HANDLE);
return;
}
@ -1004,7 +1006,7 @@ static void fs__fstat(uv_fs_t* req) {
VERIFY_FD(fd, req);
handle = (HANDLE) _get_osfhandle(fd);
handle = uv__get_osfhandle(fd);
if (handle == INVALID_HANDLE_VALUE) {
SET_REQ_WIN32_ERROR(req, ERROR_INVALID_HANDLE);
@ -1037,7 +1039,7 @@ INLINE static void fs__sync_impl(uv_fs_t* req) {
VERIFY_FD(fd, req);
result = FlushFileBuffers((HANDLE) _get_osfhandle(fd)) ? 0 : -1;
result = FlushFileBuffers(uv__get_osfhandle(fd)) ? 0 : -1;
if (result == -1) {
SET_REQ_WIN32_ERROR(req, GetLastError());
} else {
@ -1065,7 +1067,7 @@ static void fs__ftruncate(uv_fs_t* req) {
VERIFY_FD(fd, req);
handle = (HANDLE)_get_osfhandle(fd);
handle = uv__get_osfhandle(fd);
eof_info.EndOfFile.QuadPart = req->offset;
@ -1145,7 +1147,7 @@ static void fs__fchmod(uv_fs_t* req) {
VERIFY_FD(fd, req);
handle = (HANDLE) _get_osfhandle(fd);
handle = uv__get_osfhandle(fd);
nt_status = pNtQueryInformationFile(handle,
&io_status,
@ -1226,7 +1228,7 @@ static void fs__futime(uv_fs_t* req) {
HANDLE handle;
VERIFY_FD(fd, req);
handle = (HANDLE) _get_osfhandle(fd);
handle = uv__get_osfhandle(fd);
if (handle == INVALID_HANDLE_VALUE) {
SET_REQ_WIN32_ERROR(req, ERROR_INVALID_HANDLE);

View File

@ -23,6 +23,7 @@
#define UV_WIN_HANDLE_INL_H_
#include <assert.h>
#include <io.h>
#include "uv.h"
#include "internal.h"
@ -161,4 +162,18 @@ INLINE static void uv_process_endgames(uv_loop_t* loop) {
}
}
INLINE static HANDLE uv__get_osfhandle(int fd)
{
/* _get_osfhandle() raises an assert in debug builds if the FD is invalid. */
/* But it also correctly checks the FD and returns INVALID_HANDLE_VALUE */
/* for invalid FDs in release builds (or if you let the assert continue). */
/* So this wrapper function disables asserts when calling _get_osfhandle. */
HANDLE handle;
UV_BEGIN_DISABLE_CRT_ASSERT();
handle = (HANDLE) _get_osfhandle(fd);
UV_END_DISABLE_CRT_ASSERT();
return handle;
}
#endif /* UV_WIN_HANDLE_INL_H_ */

View File

@ -36,7 +36,7 @@ uv_handle_type uv_guess_handle(uv_file file) {
return UV_UNKNOWN_HANDLE;
}
handle = (HANDLE) _get_osfhandle(file);
handle = uv__get_osfhandle(file);
switch (GetFileType(handle)) {
case FILE_TYPE_CHAR:

View File

@ -35,6 +35,25 @@
# define INLINE inline
#endif
#ifdef _DEBUG
extern __declspec( thread ) int uv__crt_assert_enabled;
#define UV_BEGIN_DISABLE_CRT_ASSERT() \
{ \
int uv__saved_crt_assert_enabled = uv__crt_assert_enabled; \
uv__crt_assert_enabled = FALSE;
#define UV_END_DISABLE_CRT_ASSERT() \
uv__crt_assert_enabled = uv__saved_crt_assert_enabled; \
}
#else
#define UV_BEGIN_DISABLE_CRT_ASSERT()
#define UV_END_DISABLE_CRT_ASSERT()
#endif
/*
* Handles
* (also see handle-inl.h)

View File

@ -1727,7 +1727,7 @@ static void eof_timer_close_cb(uv_handle_t* handle) {
int uv_pipe_open(uv_pipe_t* pipe, uv_file file) {
HANDLE os_handle = (HANDLE)_get_osfhandle(file);
HANDLE os_handle = uv__get_osfhandle(file);
if (os_handle == INVALID_HANDLE_VALUE ||
uv_set_pipe_handle(pipe->loop, pipe, os_handle, 0) == -1) {

View File

@ -482,7 +482,7 @@ static int uv__slow_poll_close(uv_loop_t* loop, uv_poll_t* handle) {
int uv_poll_init(uv_loop_t* loop, uv_poll_t* handle, int fd) {
return uv_poll_init_socket(loop, handle, (SOCKET) _get_osfhandle(fd));
return uv_poll_init_socket(loop, handle, (SOCKET) uv__get_osfhandle(fd));
}

View File

@ -26,6 +26,7 @@
#include "uv.h"
#include "internal.h"
#include "handle-inl.h"
/*
@ -230,7 +231,7 @@ static int uv__duplicate_fd(uv_loop_t* loop, int fd, HANDLE* dup) {
return ERROR_INVALID_HANDLE;
}
handle = (HANDLE) _get_osfhandle(fd);
handle = uv__get_osfhandle(fd);
return uv__duplicate_handle(loop, handle, dup);
}