diff options
author | monojenkins <jo.shields+jenkins@xamarin.com> | 2021-07-14 12:52:38 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-07-14 12:52:38 +0300 |
commit | 8dba54da1a85a55a7063945dec0c891b0c31e810 (patch) | |
tree | 3bd920fdb8292c4620cf4bfe5fcb9d929043fc15 /mono | |
parent | e44d84d10a0a6bfe8376cec30582c5c93fd3a92f (diff) |
[mono] Fix race during mono_image_storage_open (#21142)
* The mono_refcount_inc call in mono_image_storage_trypublish or
mono_image_storage_tryaddref may abort when racing against a
mono_image_storage_dtor that already decremented the refcount.
This race triggered in some cases when building aspnetcore using a Mono-based dotnet host SDK. The problem is that `mono_image_storage_close` runs outside the `mono_images_storage_lock` (and this may be unavoidable due to lock ordering concerns). Therefore, we can have a refcount that was already decremented to zero, but before `mono_image_storage_dtor` finishes removing the object from `images_storage_hash`, a parallel `mono_image_storage_trypublish` may have retrieved it from there. In that case, the `mono_refcount_inc` call will abort.
Fixed by detecting that case via `mono_refcount_tryinc` instead, and simply treating the object as if it had already been removed. It will in time actually get removed, either by the parallel `mono_image_storage_dtor`, or else by the `g_hash_table_insert` in `mono_image_storage_trypublish` (which will safely replace it with the new object, and `mono_image_storage_dtor` will then detect that and skip removal).
Co-authored-by: uweigand <uweigand@users.noreply.github.com>
Diffstat (limited to 'mono')
-rw-r--r-- | mono/metadata/image.c | 10 |
1 files changed, 8 insertions, 2 deletions
diff --git a/mono/metadata/image.c b/mono/metadata/image.c index 757324eabd4..b0733c7e14e 100644 --- a/mono/metadata/image.c +++ b/mono/metadata/image.c @@ -1592,8 +1592,11 @@ mono_image_storage_trypublish (MonoImageStorage *candidate, MonoImageStorage **o gboolean result; mono_images_storage_lock (); MonoImageStorage *val = (MonoImageStorage *)g_hash_table_lookup (images_storage_hash, candidate->key); + if (val && !mono_refcount_tryinc (val)) { + // We raced against a mono_image_storage_dtor in progress. + val = NULL; + } if (val) { - mono_refcount_inc (val); *out_storage = val; result = FALSE; } else { @@ -1624,8 +1627,11 @@ mono_image_storage_tryaddref (const char *key, MonoImageStorage **found) gboolean result = FALSE; mono_images_storage_lock (); MonoImageStorage *val = (MonoImageStorage *)g_hash_table_lookup (images_storage_hash, key); + if (val && !mono_refcount_tryinc (val)) { + // We raced against a mono_image_storage_dtor in progress. + val = NULL; + } if (val) { - mono_refcount_inc (val); *found = val; result = TRUE; } |