Skip to content

Commit

Permalink
mono_w32handle_wait_multiple: Duplication is ok for WaitAny, exceptio…
Browse files Browse the repository at this point in the history 
…n for WaitAll.

mono#9089
  • Loading branch information
@jaykrell
jaykrell committed Jun 13, 2018
1 parent 626ab10 commit ba9fffa
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 41 deletions.
51 changes: 47 additions & 4 deletions mono/metadata/threads.c
View file
Expand Up @@ -2085,16 +2085,59 @@ ves_icall_System_Threading_WaitHandle_Wait_internal (gpointer *handles, gint32 n
// Allocate handle outside of the loop.
MonoExceptionHandle exc = MONO_HANDLE_NEW (MonoException, NULL);

// Duplicate objects are not allowed for WaitAll, but are ok with WaitAny.
// There are the following implementation choices:
// - qsort and look for adjacent equal, or bubble-sort-like and look for equal
// - handle-based or object-based
// - check for duplicate earlier or later
//
// Multiple combinations exist.
//
// CoreCLR/Windows: handle-based, sorted, checking after an error
// CoreCLR/Unix: object-based, bubble-sort-like
// Mono/Unix: object-based, sorted
// Mono/Windows: handle-based, sorted
//
// MSDN documentation is unclear as to handle-based or object-based -- probably object-based.
// For .NET to be object-based in a thin Win32-based implementation is impossible.
//
// Note that the lowest array index signaled is to be returned, so
// operating on the sorted data is incorrect, or needs an indirection to "unsort".

for (;;) {
#ifdef HOST_WIN32
DWORD win32WaitResult;
MONO_ENTER_GC_SAFE;
ret = (numhandles != 1)
? mono_w32handle_convert_wait_ret (mono_win32_wait_for_multiple_objects_ex(numhandles, handles, waitall, timeoutLeft, TRUE), numhandles)
: mono_w32handle_convert_wait_ret (mono_win32_wait_for_single_object_ex (handles [0], timeoutLeft, TRUE), 1);
win32WaitResult = (numhandles != 1)
? mono_win32_wait_for_multiple_objects_ex (numhandles, handles, waitall, timeoutLeft, TRUE)
: mono_win32_wait_for_single_object_ex (handles [0], timeoutLeft, TRUE);
MONO_EXIT_GC_SAFE;
// Duplicate detection here is based on CoreCLR.
// In particular, it is triggered by an error, and it is handled-based
// instead of object-based.
// See coreclr/src/vm/threads.cpp CheckForDuplicateHandles and its use.
if (win32WaitResult == WAIT_FAILED
&& waitAll
&& numhandles > 1
&& numhandles <= MONO_W32HANDLE_MAXIMUM_WAIT_OBJECTS
&& GetLastError () == ERROR_INVALID_PARAMETER) {
gpointer handles_sorted [MONO_W32HANDLE_MAXIMUM_WAIT_OBJECTS]; // 64
memcpy (handles_sorted, handles, numhandles * sizeof (handles [0]));
qsort (handles_sorted, numhandles, sizeof (gpointer), g_direct_equal);
for (gint32 i = 1; i < numhandles; ++i) {
if (handles_sorted [i - 1] == handles_sorted [i]) {
mono_error_set_duplicate_wait_object (error);
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER_HANDLE, "%s: handle %p is duplicated", __func__, handles_sorted [i]);
// win32WaitResult == WAIT_FAILED should suffice
// to exit function correctly.
break;
}
}
}
ret = mono_w32handle_convert_wait_ret (win32WaitResult, numhandles);
#else
/* mono_w32handle_wait_multiple optimizes the case for numhandles == 1 */
ret = mono_w32handle_wait_multiple (handles, numhandles, waitall, timeoutLeft, TRUE);
ret = mono_w32handle_wait_multiple (handles, numhandles, waitall, timeoutLeft, TRUE, error);
#endif /* HOST_WIN32 */

if (ret != MONO_W32HANDLE_WAIT_RET_ALERTED)
Expand Down
95 changes: 59 additions & 36 deletions mono/metadata/w32handle.c
View file
Expand Up @@ -13,14 +13,13 @@

#include <config.h>
#include <glib.h>

#include "w32handle.h"

#include "utils/atomic.h"
#include "utils/mono-logger-internals.h"
#include "utils/mono-proclib.h"
#include "utils/mono-threads.h"
#include "utils/mono-time.h"
#include "utils/mono-error-internals.h"

#undef DEBUG_REFS

Expand Down Expand Up @@ -48,7 +47,7 @@ static MonoCoopCond global_signal_cond;

static MonoCoopMutex scan_mutex;

static gboolean shutting_down = FALSE;
static gboolean shutting_down;

static const gchar*
mono_w32handle_ops_typename (MonoW32Type type);
Expand Down Expand Up @@ -131,6 +130,8 @@ mono_w32handle_unlock_signal_mutex (void)
void
mono_w32handle_lock (MonoW32Handle *handle_data)
{
if (!handle_data)
return;
mono_coop_mutex_lock (&handle_data->signal_mutex);
}

Expand All @@ -143,6 +144,8 @@ mono_w32handle_trylock (MonoW32Handle *handle_data)
void
mono_w32handle_unlock (MonoW32Handle *handle_data)
{
if (!handle_data)
return;
mono_coop_mutex_unlock (&handle_data->signal_mutex);
}

Expand Down Expand Up @@ -418,6 +421,9 @@ mono_w32handle_ref_core (MonoW32Handle *handle_data)
static gboolean
mono_w32handle_unref_core (MonoW32Handle *handle_data)
{
if (!handle_data)
return FALSE;

MonoW32Type type;
guint old, new;

Expand Down Expand Up @@ -598,6 +604,8 @@ mono_w32handle_lock_handles (MonoW32Handle **handles_data, gsize nhandles)
/* Lock all the handles, with backoff */
again:
for (i = 0; i < nhandles; i++) {
if (!handles_data [i])
continue;
if (!mono_w32handle_trylock (handles_data [i])) {
/* Bummer */

Expand Down Expand Up @@ -901,30 +909,30 @@ mono_w32handle_wait_one (gpointer handle, guint32 timeout, gboolean alertable)
}

MonoW32HandleWaitRet
mono_w32handle_wait_multiple (gpointer *handles, gsize nhandles, gboolean waitall, guint32 timeout, gboolean alertable)
mono_w32handle_wait_multiple (gpointer *handles, gsize nhandles, gboolean waitall, guint32 timeout, gboolean alertable, MonoError *error)
{
MonoW32HandleWaitRet ret;
gboolean alerted, poll;
gint i;
gint64 start;
MonoW32Handle *handles_data [MONO_W32HANDLE_MAXIMUM_WAIT_OBJECTS], *handles_data_sorted [MONO_W32HANDLE_MAXIMUM_WAIT_OBJECTS];
gboolean abandoned [MONO_W32HANDLE_MAXIMUM_WAIT_OBJECTS] = {0};

if (nhandles == 0)
return MONO_W32HANDLE_WAIT_RET_FAILED;

if (nhandles == 1)
return mono_w32handle_wait_one (handles [0], timeout, alertable);

alerted = FALSE;

if (nhandles > MONO_W32HANDLE_MAXIMUM_WAIT_OBJECTS) {
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER_HANDLE, "%s: too many handles: %zd",
__func__, nhandles);

return MONO_W32HANDLE_WAIT_RET_FAILED;
}

MonoW32HandleWaitRet ret;
gboolean alerted, poll;
gint i;
gint64 start;
MonoW32Handle *handles_data [MONO_W32HANDLE_MAXIMUM_WAIT_OBJECTS];
gboolean abandoned [MONO_W32HANDLE_MAXIMUM_WAIT_OBJECTS] = {0};

alerted = FALSE;

for (i = 0; i < nhandles; ++i) {
if (!mono_w32handle_lookup_and_ref (handles [i], &handles_data [i])) {
for (; i >= 0; --i)
Expand All @@ -944,24 +952,41 @@ mono_w32handle_wait_multiple (gpointer *handles, gsize nhandles, gboolean waital

return MONO_W32HANDLE_WAIT_RET_FAILED;
}

handles_data_sorted [i] = handles_data [i];
}

// Duplication is ok for WaitAny, exception for WaitAll.
// System.DuplicateWaitObjectException: Duplicate objects in argument.
// It is not obviously sensible to otherwise sort/unique the handles,
// as we have to return the lowest signaled one when multiple are signaled.
// Lowest in terms of array index in the input array.
MonoW32Handle *handles_data_sorted [MONO_W32HANDLE_MAXIMUM_WAIT_OBJECTS];
memcpy (handles_data_sorted, handles_data, nhandles * sizeof (handles_data_sorted [0]));
qsort (handles_data_sorted, nhandles, sizeof (gpointer), g_direct_equal);
for (i = 1; i < nhandles; ++i) {
if (handles_data_sorted [i - 1] == handles_data_sorted [i]) {
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER_HANDLE, "%s: handle %p is duplicated", __func__, handles_data_sorted [i]);

for (i = nhandles - 1; i >= 0; --i)
mono_w32handle_unref (handles_data [i]);

return MONO_W32HANDLE_WAIT_RET_FAILED;
if (waitall) {
mono_error_set_duplicate_wait_object (error);
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER_HANDLE, "%s: handle %p is duplicated", __func__, handles_data_sorted [i]);
ret = MONO_W32HANDLE_WAIT_RET_FAILED;
goto done;
} else {
// There is are duplicates. Remove them.
for (i = 0; i < nhandles; ++i) {
for (gsize j = i + 1; j < nhandles; ++j) {
if (handles_data [i] == handles_data [j]) {
mono_w32handle_unref (handles_data [j]);
handles_data [j] = NULL;
}
}
}
}
}
}

poll = FALSE;
for (i = 0; i < nhandles; ++i) {
if (!handles_data [i])
continue;
if (handles_data [i]->type == MONO_W32TYPE_PROCESS) {
/* Can't wait for a process handle + another handle without polling */
poll = TRUE;
Expand All @@ -982,6 +1007,8 @@ mono_w32handle_wait_multiple (gpointer *handles, gsize nhandles, gboolean waital
mono_w32handle_lock_handles (handles_data, nhandles);

for (i = 0; i < nhandles; i++) {
if (!handles_data [i])
continue;
if ((mono_w32handle_test_capabilities (handles_data [i], MONO_W32HANDLE_CAP_OWN) && mono_w32handle_ops_isowned (handles_data [i]))
|| mono_w32handle_issignalled (handles_data [i]))
{
Expand All @@ -996,6 +1023,8 @@ mono_w32handle_wait_multiple (gpointer *handles, gsize nhandles, gboolean waital

if (signalled) {
for (i = 0; i < nhandles; i++) {
if (!handles_data [i])
continue;
if (own_if_signalled (handles_data [i], &abandoned [i]) && !waitall) {
/* if we are calling WaitHandle.WaitAny, .NET only owns the first one; it matters for Mutex which
* throw AbandonedMutexException in case we owned it but didn't release it */
Expand All @@ -1018,6 +1047,8 @@ mono_w32handle_wait_multiple (gpointer *handles, gsize nhandles, gboolean waital
}

for (i = 0; i < nhandles; i++) {
if (!handles_data [i])
continue;
mono_w32handle_ops_prewait (handles_data [i]);

if (mono_w32handle_test_capabilities (handles_data [i], MONO_W32HANDLE_CAP_SPECIAL_WAIT)
Expand All @@ -1028,22 +1059,14 @@ mono_w32handle_wait_multiple (gpointer *handles, gsize nhandles, gboolean waital
}

mono_w32handle_lock_signal_mutex ();
signalled = waitall;

if (waitall) {
signalled = TRUE;
for (i = 0; i < nhandles; ++i) {
if (!mono_w32handle_issignalled (handles_data [i])) {
signalled = FALSE;
break;
}
}
} else {
signalled = FALSE;
for (i = 0; i < nhandles; ++i) {
if (mono_w32handle_issignalled (handles_data [i])) {
signalled = TRUE;
break;
}
for (i = 0; i < nhandles; ++i) {
if (!handles_data [i])
continue;
if (signalled != mono_w32handle_issignalled (handles_data [i])) {
signalled = !signalled;
break;
}
}

Expand Down
3 changes: 2 additions & 1 deletion mono/metadata/w32handle.h
View file
Expand Up @@ -13,6 +13,7 @@
#endif

#include "mono/utils/mono-coop-mutex.h"
#include "mono/utils/mono-error.h"

#ifndef INVALID_HANDLE_VALUE
#define INVALID_HANDLE_VALUE (gpointer)-1
Expand Down Expand Up @@ -160,7 +161,7 @@ MonoW32HandleWaitRet
mono_w32handle_wait_one (gpointer handle, guint32 timeout, gboolean alertable);

MonoW32HandleWaitRet
mono_w32handle_wait_multiple (gpointer *handles, gsize nhandles, gboolean waitall, guint32 timeout, gboolean alertable);
mono_w32handle_wait_multiple (gpointer *handles, gsize nhandles, gboolean waitall, guint32 timeout, gboolean alertable, MonoError *error);

MonoW32HandleWaitRet
mono_w32handle_signal_and_wait (gpointer signal_handle, gpointer wait_handle, guint32 timeout, gboolean alertable);
Expand Down
6 changes: 6 additions & 0 deletions mono/utils/mono-error-internals.h
View file
Expand Up @@ -241,6 +241,12 @@ mono_error_set_null_reference (MonoError *error)
mono_error_set_generic_error (error, "System", "NullReferenceException", "");
}

static inline void
mono_error_set_duplicate_wait_object (MonoError *error)
{
mono_error_set_generic_error (error, "System", "DuplicateWaitObjectException", "Duplicate objects in argument.");
}

void
mono_error_set_argument_out_of_range (MonoError *error, const char *name);

Expand Down

0 comments on commit ba9fffa

Please sign in to comment.