From 729651260db2a2ec0fafe06586c570febe69e60d Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Thu, 7 Feb 2013 17:06:49 -0800 Subject: [PATCH 1/5] a program to print out the error strings for winsock errors --- test/Makefile.nmake | 9 ++++-- test/print-winsock-errors.c | 61 +++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 test/print-winsock-errors.c diff --git a/test/Makefile.nmake b/test/Makefile.nmake index 09b9e879..f4372b5c 100644 --- a/test/Makefile.nmake +++ b/test/Makefile.nmake @@ -24,11 +24,13 @@ REGRESS_OBJS=regress.obj regress_buffer.obj regress_http.obj regress_dns.obj \ OTHER_OBJS=test-init.obj test-eof.obj test-weof.obj test-time.obj \ bench.obj bench_cascade.obj bench_http.obj bench_httpclient.obj \ - test-changelist.obj + test-changelist.obj \ + print-winsock-errors.obj PROGRAMS=regress.exe \ test-init.exe test-eof.exe test-weof.exe test-time.exe \ - test-changelist.exe + test-changelist.exe \ + print-winsock-errors.exe # Disabled for now: # bench.exe bench_cascade.exe bench_http.exe bench_httpclient.exe @@ -52,6 +54,9 @@ test-weof.exe: test-weof.obj test-time.exe: test-time.obj $(CC) $(CFLAGS) $(LIBS) test-time.obj +print-winsock-errors.exe: print-winsock-errors.obj + $(CC) $(CFLAGS) $(LIBS) print-winsock-errors.obj + bench.exe: bench.obj $(CC) $(CFLAGS) $(LIBS) bench.obj bench_cascade.exe: bench_cascade.obj diff --git a/test/print-winsock-errors.c b/test/print-winsock-errors.c new file mode 100644 index 00000000..5064d0ba --- /dev/null +++ b/test/print-winsock-errors.c @@ -0,0 +1,61 @@ +#include +#include + +#include +#include + +#include "event2/util.h" +#include "event2/thread.h" + +#define E(x) printf (#x " -> \"%s\"\n", evutil_socket_error_to_string (x)); + +int main (int argc, char **argv) +{ + evthread_use_windows_threads (); + + E(WSAEINTR); + E(WSAEACCES); + E(WSAEFAULT); + E(WSAEINVAL); + E(WSAEMFILE); + E(WSAEWOULDBLOCK); + E(WSAEINPROGRESS); + E(WSAEALREADY); + E(WSAENOTSOCK); + E(WSAEDESTADDRREQ); + E(WSAEMSGSIZE); + E(WSAEPROTOTYPE); + E(WSAENOPROTOOPT); + E(WSAEPROTONOSUPPORT); + E(WSAESOCKTNOSUPPORT); + E(WSAEOPNOTSUPP); + E(WSAEPFNOSUPPORT); + E(WSAEAFNOSUPPORT); + E(WSAEADDRINUSE); + E(WSAEADDRNOTAVAIL); + E(WSAENETDOWN); + E(WSAENETUNREACH); + E(WSAENETRESET); + E(WSAECONNABORTED); + E(WSAECONNRESET); + E(WSAENOBUFS); + E(WSAEISCONN); + E(WSAENOTCONN); + E(WSAESHUTDOWN); + E(WSAETIMEDOUT); + E(WSAECONNREFUSED); + E(WSAEHOSTDOWN); + E(WSAEHOSTUNREACH); + E(WSAEPROCLIM); + E(WSASYSNOTREADY); + E(WSAVERNOTSUPPORTED); + E(WSANOTINITIALISED); + E(WSAEDISCON); + E(WSATYPE_NOT_FOUND); + E(WSAHOST_NOT_FOUND); + E(WSATRY_AGAIN); + E(WSANO_RECOVERY); + E(WSANO_DATA); + + return EXIT_SUCCESS; +} From 0c6ec5d816189805c6fafdf385b171cbc3f67ff3 Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Thu, 7 Feb 2013 17:20:08 -0800 Subject: [PATCH 2/5] use FormatMessage for winsock errors as discussed here: http://archives.seul.org/libevent/users/Feb-2013/msg00004.html --- event.c | 2 + evthread-internal.h | 1 + evutil.c | 191 +++++++++++++++++++++++++++++--------------- 3 files changed, 128 insertions(+), 66 deletions(-) diff --git a/event.c b/event.c index b5d9e7b8..51e34a99 100644 --- a/event.c +++ b/event.c @@ -3468,6 +3468,8 @@ event_global_setup_locks_(const int enable_locks) #endif if (evsig_global_setup_locks_(enable_locks) < 0) return -1; + if (evutil_global_setup_locks_(enable_locks) < 0) + return -1; if (evutil_secure_rng_global_setup_locks_(enable_locks) < 0) return -1; return 0; diff --git a/evthread-internal.h b/evthread-internal.h index 6de07c24..346b2bb9 100644 --- a/evthread-internal.h +++ b/evthread-internal.h @@ -373,6 +373,7 @@ void *evthread_setup_global_lock_(void *lock_, unsigned locktype, int event_global_setup_locks_(const int enable_locks); int evsig_global_setup_locks_(const int enable_locks); +int evutil_global_setup_locks_(const int enable_locks); int evutil_secure_rng_global_setup_locks_(const int enable_locks); #endif diff --git a/evutil.c b/evutil.c index 7678a562..655a5b93 100644 --- a/evutil.c +++ b/evutil.c @@ -81,6 +81,7 @@ #include "util-internal.h" #include "log-internal.h" #include "mm-internal.h" +#include "evthread-internal.h" #include "strlcpy-internal.h" #include "ipv6-internal.h" @@ -1597,80 +1598,137 @@ evutil_gai_strerror(int err) } #ifdef _WIN32 -#define E(code, s) { code, (s " [" #code " ]") } -static struct { int code; const char *msg; } windows_socket_errors[] = { - E(WSAEINTR, "Interrupted function call"), - E(WSAEACCES, "Permission denied"), - E(WSAEFAULT, "Bad address"), - E(WSAEINVAL, "Invalid argument"), - E(WSAEMFILE, "Too many open files"), - E(WSAEWOULDBLOCK, "Resource temporarily unavailable"), - E(WSAEINPROGRESS, "Operation now in progress"), - E(WSAEALREADY, "Operation already in progress"), - E(WSAENOTSOCK, "Socket operation on nonsocket"), - E(WSAEDESTADDRREQ, "Destination address required"), - E(WSAEMSGSIZE, "Message too long"), - E(WSAEPROTOTYPE, "Protocol wrong for socket"), - E(WSAENOPROTOOPT, "Bad protocol option"), - E(WSAEPROTONOSUPPORT, "Protocol not supported"), - E(WSAESOCKTNOSUPPORT, "Socket type not supported"), - /* What's the difference between NOTSUPP and NOSUPPORT? :) */ - E(WSAEOPNOTSUPP, "Operation not supported"), - E(WSAEPFNOSUPPORT, "Protocol family not supported"), - E(WSAEAFNOSUPPORT, "Address family not supported by protocol family"), - E(WSAEADDRINUSE, "Address already in use"), - E(WSAEADDRNOTAVAIL, "Cannot assign requested address"), - E(WSAENETDOWN, "Network is down"), - E(WSAENETUNREACH, "Network is unreachable"), - E(WSAENETRESET, "Network dropped connection on reset"), - E(WSAECONNABORTED, "Software caused connection abort"), - E(WSAECONNRESET, "Connection reset by peer"), - E(WSAENOBUFS, "No buffer space available"), - E(WSAEISCONN, "Socket is already connected"), - E(WSAENOTCONN, "Socket is not connected"), - E(WSAESHUTDOWN, "Cannot send after socket shutdown"), - E(WSAETIMEDOUT, "Connection timed out"), - E(WSAECONNREFUSED, "Connection refused"), - E(WSAEHOSTDOWN, "Host is down"), - E(WSAEHOSTUNREACH, "No route to host"), - E(WSAEPROCLIM, "Too many processes"), +/* destructively remove a trailing line terminator from s */ +static void +chomp (char *s) +{ + size_t len; + if (s && (len = strlen (s)) > 0 && s[len - 1] == '\n') { + s[--len] = 0; + if (len > 0 && s[len - 1] == '\r') + s[--len] = 0; + } +} - /* Yes, some of these start with WSA, not WSAE. No, I don't know why. */ - E(WSASYSNOTREADY, "Network subsystem is unavailable"), - E(WSAVERNOTSUPPORTED, "Winsock.dll out of range"), - E(WSANOTINITIALISED, "Successful WSAStartup not yet performed"), - E(WSAEDISCON, "Graceful shutdown now in progress"), -#ifdef WSATYPE_NOT_FOUND - E(WSATYPE_NOT_FOUND, "Class type not found"), -#endif - E(WSAHOST_NOT_FOUND, "Host not found"), - E(WSATRY_AGAIN, "Nonauthoritative host not found"), - E(WSANO_RECOVERY, "This is a nonrecoverable error"), - E(WSANO_DATA, "Valid name, no data record of requested type)"), +/* FormatMessage returns allocated strings, but evutil_socket_error_to_string + * is supposed to return a string which is good indefinitely without having + * to be freed. To make this work without leaking memory, we cache the + * string the first time FormatMessage is called on a particular error + * code, and then return the cached string on subsequent calls with the + * same code. The strings aren't freed until libevent_global_shutdown + * (or never). We use a linked list to cache the errors, because we + * only expect there to be a few dozen, and that should be fast enough. + */ - /* There are some more error codes whose numeric values are marked - * OS dependent. They start with WSA_, apparently for the same - * reason that practitioners of some craft traditions deliberately - * introduce imperfections into their baskets and rugs "to allow the - * evil spirits to escape." If we catch them, then our binaries - * might not report consistent results across versions of Windows. - * Thus, I'm going to let them all fall through. - */ - { -1, NULL }, +struct cached_sock_errs { + DWORD code; + char *msg; /* allocated with LocalAlloc; free with LocalFree */ + struct cached_sock_errs *next; }; -#undef E + +#ifndef EVENT__DISABLE_THREAD_SUPPORT +static void *windows_socket_errors_lock_ = NULL; +#endif + +static struct cached_sock_errs *windows_socket_errors; + /** Equivalent to strerror, but for windows socket errors. */ const char * evutil_socket_error_to_string(int errcode) { - /* XXXX Is there really no built-in function to do this? */ - int i; - for (i=0; windows_socket_errors[i].code >= 0; ++i) { - if (errcode == windows_socket_errors[i].code) - return windows_socket_errors[i].msg; - } - return strerror(errcode); + struct cached_sock_errs *errs, *newerr; + char *msg = NULL; + + EVLOCK_LOCK(windows_socket_errors_lock_, 0); + + for (errs = windows_socket_errors; errs != NULL; errs = errs->next) + if (errs->code == errcode) { + msg = errs->msg; + goto done; + } + + if (0 != FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | + FORMAT_MESSAGE_IGNORE_INSERTS | + FORMAT_MESSAGE_ALLOCATE_BUFFER, + NULL, errcode, 0, (LPTSTR)&msg, 0, NULL)) + chomp (msg); /* because message has trailing newline */ + else { + size_t len = 50; + /* use LocalAlloc because FormatMessage does */ + msg = LocalAlloc(LMEM_FIXED, len); + if (!msg) { + msg = "winsock error"; + goto done; + } + evutil_snprintf(msg, len, "winsock error 0x%08x", errcode); + } + + newerr = (struct cached_sock_errs *) + mm_malloc(sizeof (struct cached_sock_errs)); + + if (!newerr) { + LocalFree(msg); + msg = "winsock error"; + goto done; + } + + newerr->code = errcode; + newerr->msg = msg; + newerr->next = windows_socket_errors; + windows_socket_errors = newerr; + + done: + EVLOCK_UNLOCK(windows_socket_errors_lock_, 0); + + return msg; } + +#ifndef EVENT__DISABLE_THREAD_SUPPORT +int +evutil_global_setup_locks_(const int enable_locks) +{ + EVTHREAD_SETUP_GLOBAL_LOCK(windows_socket_errors_lock_, 0); + return 0; +} +#endif + +static void +evutil_free_sock_err_globals(void) +{ + struct cached_sock_errs *errs, *tofree; + + for (errs = windows_socket_errors; errs != NULL; ) { + LocalFree (errs->msg); + tofree = errs; + errs = errs->next; + mm_free (tofree); + } + + windows_socket_errors = NULL; + +#ifndef EVENT__DISABLE_THREAD_SUPPORT + if (windows_socket_errors_lock_ != NULL) { + EVTHREAD_FREE_LOCK(windows_socket_errors_lock_, 0); + windows_socket_errors_lock_ = NULL; + } +#endif +} + +#else + +#ifndef EVENT__DISABLE_THREAD_SUPPORT +int +evutil_global_setup_locks_(const int enable_locks) +{ + return 0; +} +#endif + +static void +evutil_free_sock_err_globals(void) +{ +} + #endif int @@ -2529,4 +2587,5 @@ void evutil_free_globals_(void) { evutil_free_secure_rng_globals_(); + evutil_free_sock_err_globals(); } From 2078e9b46aa25cb3131acc9e575939533a77eb36 Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Thu, 14 Feb 2013 20:14:37 -0800 Subject: [PATCH 3/5] make sure caching works, and we don't leak memory --- test/print-winsock-errors.c | 101 +++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 43 deletions(-) diff --git a/test/print-winsock-errors.c b/test/print-winsock-errors.c index 5064d0ba..d085d83d 100644 --- a/test/print-winsock-errors.c +++ b/test/print-winsock-errors.c @@ -4,6 +4,7 @@ #include #include +#include "event2/event.h" #include "event2/util.h" #include "event2/thread.h" @@ -11,51 +12,65 @@ int main (int argc, char **argv) { + int i; + const char *s1, *s2; + evthread_use_windows_threads (); - E(WSAEINTR); - E(WSAEACCES); - E(WSAEFAULT); - E(WSAEINVAL); - E(WSAEMFILE); - E(WSAEWOULDBLOCK); - E(WSAEINPROGRESS); - E(WSAEALREADY); - E(WSAENOTSOCK); - E(WSAEDESTADDRREQ); - E(WSAEMSGSIZE); - E(WSAEPROTOTYPE); - E(WSAENOPROTOOPT); - E(WSAEPROTONOSUPPORT); - E(WSAESOCKTNOSUPPORT); - E(WSAEOPNOTSUPP); - E(WSAEPFNOSUPPORT); - E(WSAEAFNOSUPPORT); - E(WSAEADDRINUSE); - E(WSAEADDRNOTAVAIL); - E(WSAENETDOWN); - E(WSAENETUNREACH); - E(WSAENETRESET); - E(WSAECONNABORTED); - E(WSAECONNRESET); - E(WSAENOBUFS); - E(WSAEISCONN); - E(WSAENOTCONN); - E(WSAESHUTDOWN); - E(WSAETIMEDOUT); - E(WSAECONNREFUSED); - E(WSAEHOSTDOWN); - E(WSAEHOSTUNREACH); - E(WSAEPROCLIM); - E(WSASYSNOTREADY); - E(WSAVERNOTSUPPORTED); - E(WSANOTINITIALISED); - E(WSAEDISCON); - E(WSATYPE_NOT_FOUND); - E(WSAHOST_NOT_FOUND); - E(WSATRY_AGAIN); - E(WSANO_RECOVERY); - E(WSANO_DATA); + s1 = evutil_socket_error_to_string (WSAEINTR); + + for (i = 0; i < 3; i++) { + printf ("\niteration %d:\n\n", i); + E(WSAEINTR); + E(WSAEACCES); + E(WSAEFAULT); + E(WSAEINVAL); + E(WSAEMFILE); + E(WSAEWOULDBLOCK); + E(WSAEINPROGRESS); + E(WSAEALREADY); + E(WSAENOTSOCK); + E(WSAEDESTADDRREQ); + E(WSAEMSGSIZE); + E(WSAEPROTOTYPE); + E(WSAENOPROTOOPT); + E(WSAEPROTONOSUPPORT); + E(WSAESOCKTNOSUPPORT); + E(WSAEOPNOTSUPP); + E(WSAEPFNOSUPPORT); + E(WSAEAFNOSUPPORT); + E(WSAEADDRINUSE); + E(WSAEADDRNOTAVAIL); + E(WSAENETDOWN); + E(WSAENETUNREACH); + E(WSAENETRESET); + E(WSAECONNABORTED); + E(WSAECONNRESET); + E(WSAENOBUFS); + E(WSAEISCONN); + E(WSAENOTCONN); + E(WSAESHUTDOWN); + E(WSAETIMEDOUT); + E(WSAECONNREFUSED); + E(WSAEHOSTDOWN); + E(WSAEHOSTUNREACH); + E(WSAEPROCLIM); + E(WSASYSNOTREADY); + E(WSAVERNOTSUPPORTED); + E(WSANOTINITIALISED); + E(WSAEDISCON); + E(WSATYPE_NOT_FOUND); + E(WSAHOST_NOT_FOUND); + E(WSATRY_AGAIN); + E(WSANO_RECOVERY); + E(WSANO_DATA); + } + + s2 = evutil_socket_error_to_string (WSAEINTR); + if (s1 != s2) + printf ("caching failed!\n"); + + libevent_global_shutdown (); return EXIT_SUCCESS; } From 4ccdd53f78b200651ba0291466551e16486def24 Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Mon, 25 Feb 2013 19:02:32 -0800 Subject: [PATCH 4/5] use hashtable instead of linked list to cache winsock errors as discussed here: https://github.com/libevent/libevent/pull/41#issuecomment-13611817 --- evutil.c | 82 ++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 62 insertions(+), 20 deletions(-) diff --git a/evutil.c b/evutil.c index 655a5b93..ce39eda1 100644 --- a/evutil.c +++ b/evutil.c @@ -87,6 +87,8 @@ #include "ipv6-internal.h" #ifdef _WIN32 +#define HT_NO_CACHE_HASH_VALUES +#include "ht-internal.h" #define open _open #define read _read #define close _close @@ -1620,32 +1622,70 @@ chomp (char *s) * only expect there to be a few dozen, and that should be fast enough. */ -struct cached_sock_errs { +struct cached_sock_errs_entry { + HT_ENTRY(cached_sock_errs_entry) node; DWORD code; char *msg; /* allocated with LocalAlloc; free with LocalFree */ - struct cached_sock_errs *next; }; +static inline unsigned +hash_cached_sock_errs(const struct cached_sock_errs_entry *e) +{ + /* Use Murmur3's 32-bit finalizer as an integer hash function */ + DWORD h = e->code; + h ^= h >> 16; + h *= 0x85ebca6b; + h ^= h >> 13; + h *= 0xc2b2ae35; + h ^= h >> 16; + return h; +} + +static inline int +eq_cached_sock_errs(const struct cached_sock_errs_entry *a, + const struct cached_sock_errs_entry *b) +{ + return a->code == b->code; +} + #ifndef EVENT__DISABLE_THREAD_SUPPORT static void *windows_socket_errors_lock_ = NULL; #endif -static struct cached_sock_errs *windows_socket_errors; +static HT_HEAD(cached_sock_errs_map, cached_sock_errs_entry) + windows_socket_errors = HT_INITIALIZER(); + +HT_PROTOTYPE(cached_sock_errs_map, + cached_sock_errs_entry, + node, + hash_cached_sock_errs, + eq_cached_sock_errs); + +HT_GENERATE(cached_sock_errs_map, + cached_sock_errs_entry, + node, + hash_cached_sock_errs, + eq_cached_sock_errs, + 0.5, + mm_malloc, + mm_realloc, + mm_free); /** Equivalent to strerror, but for windows socket errors. */ const char * evutil_socket_error_to_string(int errcode) { - struct cached_sock_errs *errs, *newerr; + struct cached_sock_errs_entry *errs, *newerr, find; char *msg = NULL; EVLOCK_LOCK(windows_socket_errors_lock_, 0); - for (errs = windows_socket_errors; errs != NULL; errs = errs->next) - if (errs->code == errcode) { - msg = errs->msg; - goto done; - } + find.code = errcode; + errs = HT_FIND(cached_sock_errs_map, &windows_socket_errors, &find); + if (errs) { + msg = errs->msg; + goto done; + } if (0 != FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | @@ -1663,8 +1703,8 @@ evutil_socket_error_to_string(int errcode) evutil_snprintf(msg, len, "winsock error 0x%08x", errcode); } - newerr = (struct cached_sock_errs *) - mm_malloc(sizeof (struct cached_sock_errs)); + newerr = (struct cached_sock_errs_entry *) + mm_malloc(sizeof (struct cached_sock_errs_entry)); if (!newerr) { LocalFree(msg); @@ -1674,8 +1714,7 @@ evutil_socket_error_to_string(int errcode) newerr->code = errcode; newerr->msg = msg; - newerr->next = windows_socket_errors; - windows_socket_errors = newerr; + HT_INSERT(cached_sock_errs_map, &windows_socket_errors, newerr); done: EVLOCK_UNLOCK(windows_socket_errors_lock_, 0); @@ -1695,16 +1734,19 @@ evutil_global_setup_locks_(const int enable_locks) static void evutil_free_sock_err_globals(void) { - struct cached_sock_errs *errs, *tofree; + struct cached_sock_errs_entry **errs, *tofree; - for (errs = windows_socket_errors; errs != NULL; ) { - LocalFree (errs->msg); - tofree = errs; - errs = errs->next; - mm_free (tofree); + for (errs = HT_START(cached_sock_errs_map, &windows_socket_errors) + ; errs; ) { + tofree = *errs; + errs = HT_NEXT_RMV(cached_sock_errs_map, + &windows_socket_errors, + errs); + LocalFree(tofree->msg); + mm_free(tofree); } - windows_socket_errors = NULL; + HT_CLEAR(cached_sock_errs_map, &windows_socket_errors); #ifndef EVENT__DISABLE_THREAD_SUPPORT if (windows_socket_errors_lock_ != NULL) { From c9ad3af229781b401360e21ff0f3e19c80a72498 Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Mon, 25 Feb 2013 20:13:01 -0800 Subject: [PATCH 5/5] test filling up the hash table a bit --- test/print-winsock-errors.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/print-winsock-errors.c b/test/print-winsock-errors.c index d085d83d..ab6e610e 100644 --- a/test/print-winsock-errors.c +++ b/test/print-winsock-errors.c @@ -12,7 +12,7 @@ int main (int argc, char **argv) { - int i; + int i, j; const char *s1, *s2; evthread_use_windows_threads (); @@ -64,6 +64,14 @@ int main (int argc, char **argv) E(WSATRY_AGAIN); E(WSANO_RECOVERY); E(WSANO_DATA); + E(0xdeadbeef); /* test the case where no message is available */ + + /* fill up the hash table a bit to make sure it grows properly */ + for (j = 0; j < 50; j++) { + int err; + evutil_secure_rng_get_bytes(&err, sizeof(err)); + evutil_socket_error_to_string(err); + } } s2 = evutil_socket_error_to_string (WSAEINTR);