search

CesiumGS / cesium-unity

Bringing the 3D geospatial ecosystem to Unity
https://cesium.com/platform/cesium-for-unity/
Apache License 2.0
358 stars 83 forks source link

Reference counting and accidental copy prevention for Impl classes #503

Closed kring closed 2 months ago

kring commented 2 months ago

Reference Counting

Previously, C# objects backed by a native "Impl" object had strict ownership of the Impl instance. It was new'd in the constructor, and delete'd in the finalizer. This was mostly fine, but there are a few cases (mostly related to Future continuations) where it's important to keep the Impl instance alive even though the C# instance might get finalized. See #499. Previously this was not really possible because of the strict ownership. In this PR, Impl instances are internally reference counted, so their lifetime can be managed with an IntrusivePointer.

Note that I haven't actually done anything to take advantage of this in this PR. This PR is just the mostly-mechanical refactoring to reference counting.

Copy prevention

This PR also makes it impossible to accidentally copy Impl classes by declaring deleted constructors and assignment operators. Accidental copying has been responsible for at least one bug in the past, and it was really simple to do this as part of the reference counting change, so I did.

Fixes #223

j9liu commented 2 months ago

Looks good to me @kring . Not sure what is the best way to test this, but at the very least I built and ran the branch, and connected to Cesium ion with no issues.