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.

Author: orangemocha

src/win/core.c

@@ -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();

src/win/fs.c

@@ -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);

src/win/handle-inl.h

@@ -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_ */

src/win/handle.c

@@ -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:

src/win/internal.h

@@ -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)

src/win/pipe.c

@@ -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) {

src/win/poll.c

@@ -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));
 }
 
 

src/win/process-stdio.c

@@ -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);
 }