Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[hot_reload] assorted hot reload fixes for cumulative property updates #85351

Merged
merged 4 commits into from
Apr 25, 2023
Merged

[hot_reload] assorted hot reload fixes for cumulative property updates #85351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
05b446a
[hot_reload] remove unneeded assertion
lambdageek Apr 25, 2023
3c0cd21
Update generation on MonoClassMetadataUpdateInfo when adding members
lambdageek Apr 25, 2023
c009f37
assert that generic instances don't call hot_reload_get_property
lambdageek Apr 25, 2023
8d4953a
update regression test
lambdageek Apr 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...pdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,6 @@ public double FireEvents() {
return Accumulator;
}

public double AddedFirstProp {get => 0.0; set { Console.WriteLine (value); } }
}
}
91 changes: 91 additions & 0 deletions ...pdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
using System;


namespace System.Reflection.Metadata.ApplyUpdate.Test
{
public class AddInstanceField
{
public AddInstanceField () {
_doubleField2 = 5.5;
_stringField2 = "New Initial Value";
NewStructField = new NewStruct {
D = -1985.0,
O = new int[2] { 15, 17 },
};
// a little bit ldflda testing
IncRefDouble (ref NewStructField.D);
IncRefDouble (ref _doubleField2);

AddedStringAutoProp = "abcd";

AddedEvent += MyHandler;

void MyHandler (object sender, double data) {
}
}

public void IncRefDouble (ref double d)
{
d += 1.0;
}

public string GetStringField => _stringField2;
public double GetDoubleField => _doubleField2;

private string _stringField;
private string _stringField2;
private double _doubleField;
private double _doubleField2;

private int[] _intArrayFieldWithInit = new[] { 2, 4, 6, 8, 10, 12 };
private int[] _intArrayFieldWithInit2 = new[] { 1, 3, 5, 7, 9, 11 };

public void TestMethod () {
_stringField = "spqr";
_stringField2 = "4567";
_doubleField = 2.71828;
_doubleField2 = 0.707106;
AddedStringAutoProp = AddedStringAutoProp + "Test";
}

public int GetIntArrayLength() => _intArrayFieldWithInit2?.Length ?? -1;
public int GetIntArrayElt(int i) => _intArrayFieldWithInit2[i];

public struct NewStruct
{
public double D;
public object O;
}

public NewStruct NewStructField;

public string GetStringProp => AddedStringAutoProp;

public string AddedStringAutoProp { get; set; }

public event EventHandler<double> ExistingEvent;
public event EventHandler<double> AddedEvent;

public double Accumulator;

private void AccumHandler (object sender, double value) => Accumulator += value;

public double FireEvents() {
Accumulator = 0.0;
ExistingEvent += AccumHandler;
ExistingEvent(this, 123.0);
ExistingEvent -= AccumHandler;

AddedEvent += AccumHandler;
AddedEvent(this, 123.0);
AddedEvent -= AccumHandler;

return Accumulator;
}

public double AddedFirstProp {get => 0.0; set { Console.WriteLine (value+value); } }
public short AddedSecondProp {get; set; }
}
}
1 change: 1 addition & 0 deletions ...ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/deltascript.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"changes": [
{"document": "AddInstanceField.cs", "update": "AddInstanceField_v1.cs"},
{"document": "AddInstanceField.cs", "update": "AddInstanceField_v2.cs"},
]
}

10 changes: 10 additions & 0 deletions src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,16 @@ public static void TestAddInstanceField()
Assert.True ((addedEventToken & 0x00ffffff) < 4);


ApplyUpdateUtil.ApplyUpdate(assm);

var addedFirstPropInfo = x2.GetType().GetProperty("AddedFirstProp");
var firstPropSetter = addedFirstPropInfo.GetSetMethod();
Assert.NotNull (firstPropSetter);

var addedSecondPropInfo = x2.GetType().GetProperty("AddedSecondProp");
var secondPropGetter = addedSecondPropInfo.GetGetMethod();
Assert.NotNull (secondPropGetter);

});
}

Expand Down
19 changes: 9 additions & 10 deletions src/mono/mono/component/hot_reload.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2305,10 +2305,7 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base
}
case MONO_TABLE_PROPERTY: {
/* allow updates to existing properties. */
if (!is_addition)
/* FIXME: use DeltaInfo:prev_gen_rows instead of image_base */
g_assert (token_index <= table_info_get_rows (&image_base->tables [token_table]));
else {
if (is_addition) {
g_assert (add_property_propertymap != 0);

uint32_t parent_type_token = mono_metadata_decode_row_col (&image_base->tables [MONO_TABLE_PROPERTYMAP], mono_metadata_token_index (add_property_propertymap) - 1, MONO_PROPERTY_MAP_PARENT);
Expand Down Expand Up @@ -2362,10 +2359,7 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base

}
case MONO_TABLE_EVENT: {
if (!is_addition)
/* FIXME: use DeltaInfo:prev_gen_rows instead of image_base */
g_assert (token_index <= table_info_get_rows (&image_base->tables [token_table]));
else {
if (is_addition) {
g_assert (add_event_eventmap != 0);

uint32_t parent_type_token = mono_metadata_decode_row_col (&image_base->tables [MONO_TABLE_EVENTMAP], mono_metadata_token_index (add_event_eventmap) - 1, MONO_EVENT_MAP_PARENT);
Expand Down Expand Up @@ -2869,6 +2863,7 @@ add_member_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClas
GSList *members = klass_info->added_members;
klass_info->added_members = g_slist_prepend_mem_manager (m_class_get_mem_manager (klass), members, GUINT_TO_POINTER (member_token));
add_member_parent (base_info, m_class_get_type_token (klass), member_token);
klass_info->generation = delta_info->generation;
}


Expand Down Expand Up @@ -3025,6 +3020,7 @@ hot_reload_get_field (MonoClass *klass, uint32_t fielddef_token) {
static MonoProperty *
hot_reload_get_property (MonoClass *klass, uint32_t property_token)
{
g_assert (m_class_get_class_kind (klass) != MONO_CLASS_GINST);
MonoClassMetadataUpdateInfo *info = mono_class_get_or_add_metadata_update_info (klass);
g_assert (mono_metadata_token_table (property_token) == MONO_TABLE_PROPERTY);
GSList *added_props = info->added_props;
Expand All @@ -3040,6 +3036,7 @@ hot_reload_get_property (MonoClass *klass, uint32_t property_token)
static MonoEvent *
hot_reload_get_event (MonoClass *klass, uint32_t event_token)
{
g_assert (m_class_get_class_kind (klass) != MONO_CLASS_GINST);
MonoClassMetadataUpdateInfo *info = mono_class_get_or_add_metadata_update_info (klass);
g_assert (mono_metadata_token_table (event_token) == MONO_TABLE_EVENT);
GSList *added_events = info->added_events;
Expand Down Expand Up @@ -3108,6 +3105,8 @@ add_property_to_existing_class (MonoImage *image_base, BaselineInfo *base_info,

parent_info->added_props = g_slist_prepend_mem_manager (m_class_get_mem_manager (parent_klass), parent_info->added_props, prop);

parent_info->generation = generation;

return prop;

}
Expand All @@ -3134,6 +3133,8 @@ add_event_to_existing_class (MonoImage *image_base, BaselineInfo *base_info, uin

parent_info->added_events = g_slist_prepend_mem_manager (m_class_get_mem_manager (parent_klass), parent_info->added_events, evt);

parent_info->generation = generation;

return evt;

}
Expand Down Expand Up @@ -3167,7 +3168,6 @@ add_semantic_method_to_existing_property (MonoImage *image_base, BaselineInfo *b


g_assert (dest != NULL);
g_assert (*dest == NULL);

MonoMethod *method = mono_get_method_checked (image_base, method_token, klass, NULL, error);
mono_error_assert_ok (error);
Expand Down Expand Up @@ -3207,7 +3207,6 @@ add_semantic_method_to_existing_event (MonoImage *image_base, BaselineInfo *base
}

g_assert (dest != NULL);
g_assert (*dest == NULL);

MonoMethod *method = mono_get_method_checked (image_base, method_token, klass, NULL, error);
mono_error_assert_ok (error);
Expand Down