Skip to content

Commit

Permalink
[2018-10][marshal] Emit GC Safe transitions around foreign internal c…
Browse files Browse the repository at this point in the history
…alls (mono#11184)

* [threads] Make mono_add_internal_call external only

The runtime should use mono_add_internal_call_internal or
mono_add_internal_call_with_flags.

The thread suspend mechanism needs to know if an icall is added via the legacy
mono_add_internal_call API (which is not coop-aware, and so the registered
icalls must run in GC Safe mode), or if it is added by the runtime or by a
cooperative client (currently either the profiler or the System.Native PAL)
which knows to add GC transitions and safepoints and not to block indefinitely.

* [marshal] Factor out GCSafeTransitionBuilder

Make a local builder for creating the GC Safe transition calls for a method wraper.

* [sgen] Add coop GC transitions in mono_gc_toggleref_add

Also mark it external only. It's not used inside the runtime.

* [marshal] Emit GC Safe transitions around foreign icalls.

A foreign icall is added using mono_add_internal_call and that is not coop GC
aware.  (The runtime uses mono_add_internal_call_with_flags or
mono_add_internal_call_internal)

Under hybrid suspend, foreign icalls will run in GC Safe mode and transition to
GC Unsafe only when the call back into the runtime or invoke managed code.

Fixes mono#11138

* [cxx] function type overloads for mono_add_internal_call_internal

and mono_add_internal_call_with_flags

* [runtime] Add new API call mono_dangerous_add_raw_internal_call

Under hybrid suspend, this adds an icall that is assumed to run in GC Unsafe
mode.  As such it has additional requirements for correct operation: it must
not run loops without periodically polling the runtime, and it must not perform
blocking operations such as blocking I/O or taking locks without manually
switching to GC Safe mode.
  • Loading branch information
lambdageek authored and marek-safar committed Oct 22, 2018
1 parent 90fe086 commit bbcc3eb
Show file tree
Hide file tree
Showing 13 changed files with 280 additions and 80 deletions.
31 changes: 30 additions & 1 deletion mono/metadata/icall-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,35 @@ mono_icall_drive_info_get_drive_type (MonoString *root_path_name);
#endif /* !G_HAVE_API_SUPPORT(HAVE_CLASSIC_WINAPI_SUPPORT) */

void*
mono_lookup_internal_call_full (MonoMethod *method, gboolean warn_on_missing, mono_bool *uses_handles);
mono_lookup_internal_call_full (MonoMethod *method, gboolean warn_on_missing, mono_bool *uses_handles, mono_bool *foreign);


MONO_PAL_API void
mono_add_internal_call_with_flags (const char *name, const void* method, gboolean cooperative);

MONO_PROFILER_API void
mono_add_internal_call_internal (const char *name, gconstpointer method);

#ifdef __cplusplus

#include <type_traits>

template <typename T>
inline typename std::enable_if<std::is_function<T>::value ||
std::is_function<typename std::remove_pointer<T>::type>::value >::type
mono_add_internal_call_with_flags (const char *name, T method, gboolean cooperative)
{
return mono_add_internal_call_with_flags (name, (const void*)method, cooperative);
}

template <typename T>
inline typename std::enable_if<std::is_function<T>::value ||
std::is_function<typename std::remove_pointer<T>::type>::value >::type
mono_add_internal_call_internal (const char *name, T method)
{
return mono_add_internal_call_internal (name, (const void*)method);
}

#endif // __cplusplus

#endif /* __MONO_METADATA_ICALL_INTERNALS_H__ */
86 changes: 82 additions & 4 deletions mono/metadata/icall.c
Original file line number Diff line number Diff line change
Expand Up @@ -8266,6 +8266,7 @@ ves_icall_System_IO_LogcatTextWriter_Log (const char *appname, gint32 level, con
static MonoIcallTableCallbacks icall_table;
static mono_mutex_t icall_mutex;
static GHashTable *icall_hash = NULL;
static GHashTable *icall_hash_foreign = NULL;
static GHashTable *jit_icall_hash_name = NULL;
static GHashTable *jit_icall_hash_addr = NULL;

Expand All @@ -8283,6 +8284,7 @@ mono_icall_init (void)
mono_icall_table_init ();
#endif
icall_hash = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
icall_hash_foreign = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
mono_os_mutex_init (&icall_mutex);
}

Expand All @@ -8302,6 +8304,7 @@ void
mono_icall_cleanup (void)
{
g_hash_table_destroy (icall_hash);
g_hash_table_destroy (icall_hash_foreign);
g_hash_table_destroy (jit_icall_hash_name);
g_hash_table_destroy (jit_icall_hash_addr);
mono_os_mutex_destroy (&icall_mutex);
Expand Down Expand Up @@ -8346,14 +8349,69 @@ mono_icall_cleanup (void)
*/
void
mono_add_internal_call (const char *name, gconstpointer method)
{
mono_add_internal_call_with_flags (name, method, FALSE);
}

/**
* mono_dangerous_add_raw_internal_call:
* \param name method specification to surface to the managed world
* \param method pointer to a C method to invoke when the method is called
*
* Similar to \c mono_add_internal_call but with more requirements for correct
* operation.
*
* A thread running a dangerous raw internal call will avoid a thread state
* transition on entry and exit, but it must take responsiblity for cooperating
* with the Mono runtime.
*
* The \p method must NOT:
*
* Run for an unbounded amount of time without calling the mono runtime.
* Additionally, the method must switch to GC Safe mode to perform all blocking
* operations: performing blocking I/O, taking locks, etc.
*
*/
void
mono_dangerous_add_raw_internal_call (const char *name, gconstpointer method)
{
mono_add_internal_call_with_flags (name, method, TRUE);
}

/**
* mono_add_internal_call_with_flags:
* \param name method specification to surface to the managed world
* \param method pointer to a C method to invoke when the method is called
* \param cooperative if \c TRUE, run icall in GC Unsafe (cooperatively suspended) mode,
* otherwise GC Safe (blocking)
*
* Like \c mono_add_internal_call, but if \p cooperative is \c TRUE the added
* icall promises that it will use the coopertive API to inform the runtime
* when it is running blocking operations, that it will not run for unbounded
* amounts of time without safepointing, and that it will not hold managed
* object references across suspend safepoints.
*
* If \p cooperative is \c FALSE, run the icall in GC Safe mode - the icall may
* block. The icall must obey the GC Safe rules, e.g. it must not touch
* unpinned managed memory.
*
*/
void
mono_add_internal_call_with_flags (const char *name, gconstpointer method, gboolean cooperative)
{
mono_icall_lock ();

g_hash_table_insert (icall_hash, g_strdup (name), (gpointer) method);
g_hash_table_insert (cooperative ? icall_hash : icall_hash_foreign , g_strdup (name), (gpointer) method);

mono_icall_unlock ();
}

void
mono_add_internal_call_internal (const char *name, gconstpointer method)
{
mono_add_internal_call_with_flags (name, method, TRUE);
}

/*
* we should probably export this as an helper (handle nested types).
* Returns the number of chars written in buf.
Expand Down Expand Up @@ -8392,7 +8450,7 @@ no_icall_table (void)
* If the method is not found, warns and returns NULL.
*/
gpointer
mono_lookup_internal_call_full (MonoMethod *method, gboolean warn_on_missing, mono_bool *uses_handles)
mono_lookup_internal_call_full (MonoMethod *method, gboolean warn_on_missing, mono_bool *uses_handles, mono_bool *foreign)
{
char *sigstart;
char *tmpsig;
Expand All @@ -8403,6 +8461,8 @@ mono_lookup_internal_call_full (MonoMethod *method, gboolean warn_on_missing, mo

if (uses_handles)
*uses_handles = FALSE;
if (foreign)
*foreign = FALSE;

g_assert (method != NULL);

Expand Down Expand Up @@ -8458,6 +8518,15 @@ mono_lookup_internal_call_full (MonoMethod *method, gboolean warn_on_missing, mo
mono_icall_unlock ();
return res;
}
res = g_hash_table_lookup (icall_hash_foreign, mname);
if (res) {
if (foreign)
*foreign = TRUE;
g_free (classname);
mono_icall_unlock ();
return res;
}

/* try without signature */
*sigstart = 0;
res = g_hash_table_lookup (icall_hash, mname);
Expand All @@ -8466,6 +8535,15 @@ mono_lookup_internal_call_full (MonoMethod *method, gboolean warn_on_missing, mo
mono_icall_unlock ();
return res;
}
res = g_hash_table_lookup (icall_hash_foreign, mname);
if (res) {
if (foreign)
*foreign = TRUE;
g_free (classname);
mono_icall_unlock ();
return res;
}


if (!icall_table.lookup) {
mono_icall_unlock ();
Expand Down Expand Up @@ -8505,7 +8583,7 @@ mono_lookup_internal_call_full (MonoMethod *method, gboolean warn_on_missing, mo
gpointer
mono_lookup_internal_call (MonoMethod *method)
{
return mono_lookup_internal_call_full (method, TRUE, NULL);
return mono_lookup_internal_call_full (method, TRUE, NULL, NULL);
}

/*
Expand All @@ -8520,7 +8598,7 @@ mono_lookup_icall_symbol (MonoMethod *m)
return NULL;

gpointer func;
func = mono_lookup_internal_call_full (m, FALSE, NULL);
func = mono_lookup_internal_call_full (m, FALSE, NULL, NULL);
if (!func)
return NULL;
return icall_table.lookup_icall_symbol (func);
Expand Down
11 changes: 0 additions & 11 deletions mono/metadata/loader-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,4 @@
#ifndef _MONO_METADATA_LOADER_INTERNALS_H_
#define _MONO_METADATA_LOADER_INTERNALS_H_

#ifdef __cplusplus

template <typename T>
inline void
mono_add_internal_call (const char *name, T method)
{
return mono_add_internal_call (name, (const void*)method);
}

#endif // __cplusplus

#endif
5 changes: 4 additions & 1 deletion mono/metadata/loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,12 @@ mono_method_get_flags (MonoMethod *method, uint32_t *iflags);
MONO_API uint32_t
mono_method_get_index (MonoMethod *method);

MONO_API void
MONO_API MONO_RT_EXTERNAL_ONLY void
mono_add_internal_call (const char *name, const void* method);

MONO_API MONO_RT_EXTERNAL_ONLY void
mono_dangerous_add_raw_internal_call (const char *name, const void* method);

MONO_API void*
mono_lookup_internal_call (MonoMethod *method);

Expand Down
Loading

0 comments on commit bbcc3eb

Please sign in to comment.