Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
5 changes: 3 additions & 2 deletions src/Neo.VM/Types/Array.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,11 @@ public Array(IReferenceCounter? referenceCounter, IEnumerable<StackItem>? items
foreach (var item in _array)
{
if (item is CompoundType { ReferenceCounter: null })
{
throw new InvalidOperationException("Can not set a CompoundType without a ReferenceCounter.");
}
}

foreach (var item in _array)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't use the same foreach?

Copy link
Contributor Author

@Wi1l-B0t Wi1l-B0t Jun 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't use the same foreach?

For example:
The _array has 2 items, and the first added, but the second is invalid.
Line 86 throws a InvalidOperationException, and then the ReferenceCounter is wrong.

Copy link
Contributor Author

@Wi1l-B0t Wi1l-B0t Jun 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't use the same foreach?

This issue shouldn't be triggered in real environments, but it’s better to be cautious.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If throw an exceptiotge execution is halted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If throw an exceptiotge execution is halted

Make it more rigorous.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If throw an exceptiotge execution is halted

Make it more rigorous.

Slower without need

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If throw an exceptiotge execution is halted

Make it more rigorous.

Slower without need

It's so trivial.
Few items in array in most cases.
And after the first foreach, the code will be hot(cache and branch predictor), it should be fast enough.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need, it will be a fault, have no sense

{
referenceCounter.AddReference(item, this);
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/Neo.VM/Types/Map.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ public StackItem this[PrimitiveType key]
if (IsReadOnly) throw new InvalidOperationException("The map is readonly, can not set value.");
if (ReferenceCounter != null)
{
if (value is CompoundType { ReferenceCounter: null })
throw new InvalidOperationException("Can not set value to Map(with ReferenceCounter) without ReferenceCounter.");

if (dictionary.TryGetValue(key, out StackItem? old_value))
ReferenceCounter.RemoveReference(old_value, this);
else
ReferenceCounter.AddReference(key, this);
if (value is CompoundType { ReferenceCounter: null })
{
throw new InvalidOperationException("Can not set a Map without a ReferenceCounter.");
}

ReferenceCounter.AddReference(value, this);
}
dictionary[key] = value;
Expand Down
31 changes: 31 additions & 0 deletions tests/Neo.VM.Tests/UT_ReferenceCounter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -257,5 +257,36 @@ public void TestInvalidReferenceStackItem()

Assert.ThrowsExactly<InvalidOperationException>(() => arr.Add(arr2));
}

[TestMethod]
public void TestInvalidMapSet()
{
{
var reference = new ReferenceCounter();
var map = new Map(reference);
Assert.AreEqual(0, reference.Count); // map is zero-referred

map["test"] = new Array(reference);
Assert.AreEqual(2, reference.Count); // one key + one value

Assert.ThrowsExactly<InvalidOperationException>(() => map["test"] = new Array());
Assert.AreEqual(2, reference.Count); // not changed after invalid set

map["test"] = new Integer(1); // overwrite the value
Assert.AreEqual(2, reference.Count); // not changed after overwrite
}


{
var reference = new ReferenceCounter();
var map = new Map();
Assert.ThrowsExactly<InvalidOperationException>(() => new Array(reference, [map]));

map = new Map(reference);
var array = new Array(reference, [map]);
Assert.AreEqual(1, reference.Count);
Assert.AreEqual(1, array.Count);
}
}
}
}