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.

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

Memory Leak #2677

Open
xiaoliangde opened this issue Jun 19, 2024 · 10 comments
Open

Memory Leak #2677

xiaoliangde opened this issue Jun 19, 2024 · 10 comments
Labels

Comments

@xiaoliangde
Copy link

xiaoliangde commented Jun 19, 2024

avalonia 11.0.6
mapsui 4.1.2
windows 11

if write by control's member that can't be execute destructor function(like annotation). now i use "attach" to reduce the problem.

public sealed class ViewGroupMapSui : ViewGroupBase<ContentControl>
{
    public override bool IsFinishedConstruct { get; protected set; }
    public PropertyValued<Vector2d> Center => GetPropertyEntry<PropertyValued<Vector2d>>(nameof(Center));
    public override bool CanSelectMultiViewNodes => false;
    public override PositionOptions PositionOption => PositionOptions.CoordinateMap2d;
    public override Vector2d SpaceToScreenCoordinate(PositionWorld position)
    {
        var temp = _mapControl.Map.Navigator.Viewport.WorldToScreenXY(position.X, position.Y);
        return new Vector2d(temp.screenX, temp.screenY);
    }

    public override PositionWorld ScreenToSpaceCoordinate(Vector2d position)
    {
        var temp = _mapControl.Map.Navigator.Viewport.ScreenToWorldXY(position.X, position.Y);
        return new PositionWorld(temp.worldX, temp.worldY,0);
    }

    private  MapControl _mapControl;
    public ViewGroupMapSui(XElement dom = null) : base(dom)
    {
        // _mapControl = new MapControl();
        //
        // var map = new Map();
        //
        // //string urlTemplate = "https://t0.tianditu.gov.cn/img_w/wmts?SERVICE=WMTS&REQUEST=GetTile&VERSION=1.0.0&LAYER=img&STYLE=default&TILEMATRIXSET=w&FORMAT=tiles&TILEMATRIX={z}&TILEROW={y}&TILECOL={x}&tk=你的天地图密钥";
        // //string urlTemplate2 = "https://webrd01.is.autonavi.com/appmaptile?lang=zh_cn&size=1&scale=1&style=8&x={x}&y={y}&z={z}";
        // //var tileLayer2 = new TileLayer(new HttpTileSource(new GlobalSphericalMercator(),urlTemplate2));
        // //map.Layers.Add(tileLayer2);
        //
        //
        // var tileLayer = Mapsui.Tiling.OpenStreetMap.CreateTileLayer();
        // _mapControl.Map?.Layers.Add(tileLayer);
        // SelfContentView.Content = _mapControl;
        // // var customerLayer = new Layer();
        // // mapControl.Map?.Layers.Add(customerLayer);
        // // mapControl.Map.DataChanged += MapOnDataChanged;
        // _mapControl.Map.RefreshGraphicsRequest += MapOnRefreshGraphicsRequest;
        // _mapControl.Map.ViewportInitialized += MapOnViewportInitialized;
        // // _mapControl.Map.Navigator.CenterOn(new MPoint(104.06, 30.67));
        // //_mapControl.Map.Navigator.CenterOnAndZoomTo(new MPoint(104.06, 30.67),9);
        // //map.Home = navigator => navigator.CenterOn(104.06, 30.67);
        // Center.PropertyChanged += CenterOnPropertyChanged;
        IsFinishedConstruct = true;
    }

    protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e)
    {
        base.OnAttachedToVisualTree(e);

        _mapControl = new MapControl();
        var tileLayer = Mapsui.Tiling.OpenStreetMap.CreateTileLayer();
        _mapControl.Map?.Layers.Add(tileLayer);
        SelfContentView.Content = _mapControl;
        _mapControl.Map.RefreshGraphicsRequest += MapOnRefreshGraphicsRequest;
        _mapControl.Map.ViewportInitialized += MapOnViewportInitialized;
        Center.PropertyChanged += CenterOnPropertyChanged;
        IsFinishedConstruct = true;
    }

    protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e)
    {
        SelfContentView.Content = null;
        _mapControl?.Dispose();
        _mapControl = null;
        base.OnDetachedFromVisualTree(e);
    }

    private void MapOnViewportInitialized(object sender, EventArgs e)
    {
        _mapControl.Map.Navigator.ZoomTo(5);
        _mapControl.Map.Navigator.CenterOn(new MPoint(104.06, 30.67));
    }

    void CenterOnPropertyChanged(object sender, PropertyChangedEventArgs e)
    {
        Trace.WriteLine($"CenterOnPropertyChanged : {e.PropertyName}");
    }

    private void MapOnRefreshGraphicsRequest(object sender, EventArgs e)
    {
        Dispatcher.UIThread.InvokeAsync(InvalidateChildren);
    }
}
@pauldendulk
Copy link
Member

What does the code do? Is this your solution? Or is this to demonstrate the memory leak? How does it show that there is a memory leak?

@xiaoliangde
Copy link
Author

What does the code do? Is this your solution? Or is this to demonstrate the memory leak? How does it show that there is a memory leak?

—— can't be execute destructor function(like annotation). But if others or this one not contain the mapControl that can execute desctructor. so now i create mapControl on "OnAttachedToVisualTree" to reduce the problem.

@xiaoliangde
Copy link
Author

xiaoliangde commented Jun 20, 2024

What does the code do? Is this your solution? Or is this to demonstrate the memory leak? How does it show that there is a memory leak?

to simple test the problem like this:

class MapControlEx: MapControl
{
    public MapControlEx()
    {
        var tileLayer = Mapsui.Tiling.OpenStreetMap.CreateTileLayer();
        Map?.Layers.Add(tileLayer);
    }

    ~MapControlEx()
    {
        Dispose();
        Trace.WriteLine(GetType().FullName);
    }
}

class ControlEx : ContentControl
{
    ~ControlEx()
    {
        Trace.WriteLine(GetType().FullName);
    }
}

image
image

wo can see the MapControl can't execute the destructor. maybe the control member reference some global variant to cause the problem.
Is there any remedy available on this version to this issue?

@xiaoliangde
Copy link
Author

xiaoliangde commented Jun 20, 2024

What does the code do? Is this your solution? Or is this to demonstrate the memory leak? How does it show that there is a memory leak?

guess other :

class MapControlEx : MapControl
{
    public MapControlEx()
    {
        var tileLayer = Mapsui.Tiling.OpenStreetMap.CreateTileLayer();
        Map?.Layers.Add(tileLayer);
    }

    ~MapControlEx()
    {
        Trace.WriteLine(GetType().FullName);
    }
}

class ControlEx : ContentControl
{
    ~ControlEx()
    {
        Trace.WriteLine(GetType().FullName);
    }
}

image
image

can't execute the "~MapControlEx()" too.

@pauldendulk
Copy link
Member

I created a memory leak test like we had for Xamarin.Forms and it is green #2678. I have to investigate your kind of testing next.

@pauldendulk
Copy link
Member

I looked into this a bit more. Not completely sure what exactly the problem is your struggling with. My guess is that you do not see an increase in memory use but you want to the finalizer to be called. The MapControl implements IDispose however and you should call Dispose() on it. In the MapControl.Dispose method we call GC.SuppressFinalize(this). So, I you want to cleanup something override the MapControl.Dispose().

Also I wonder if it is a good idea to inherit from the MapControl. Nowadays I am more inclined to make classes sealed because it is hard to correctly design for inheritance, and it is hard to implement a derived class because you have to guess how the parent class works.

@xiaoliangde
Copy link
Author

I looked into this a bit more. Not completely sure what exactly the problem is your struggling with. My guess is that you do not see an increase in memory use but you want to the finalizer to be called. The MapControl implements IDispose however and you should call Dispose() on it. In the MapControl.Dispose method we call GC.SuppressFinalize(this). So, I you want to cleanup something override the MapControl.Dispose().

Also I wonder if it is a good idea to inherit from the MapControl. Nowadays I am more inclined to make classes sealed because it is hard to correctly design for inheritance, and it is hard to implement a derived class because you have to guess how the parent class works.

Pay attention to the test in the last screenshot. The use of inheritance here is only to simplify the proof that it has not been released by GC. During the usage process (starting from the constructor), I aimed to detect the release of static and dynamic resource management, and then discovered that the view tree could not be cleaned up. Then we investigated various controls and found that there was an issue with locating the map control. As it is an editor (with a history it maybe reuse), it has to be written to the "AttachViusalTree" event and set to null during “DettachVisualTree” so that the parent control can be released and the impact range can be reduced. I will continue to observe the impact of this issue in the future. Thank you for your reply.

@pauldendulk
Copy link
Member

If you can provide a minimal reproducible sample I will further investigate this.

@xiaoliangde
Copy link
Author

If you can provide a minimal reproducible sample I will further investigate this.

public partial class MainWindow : Window
{
    class MapControlEx:MapControl
    {
        ~MapControlEx()
        {
            Console.WriteLine($"------  Destruct:{GetType().Name}");
        }
    }
    class ControlEx : ContentControl
    {
        ~ControlEx()
        {
            Console.WriteLine($"------  Destruct:{GetType().Name}");
        }
    }

    class ControlOnDisposeWithMap00:ContentControl,IDisposable
    {
        private MapControl _cMapControl;
        public ControlOnDisposeWithMap00()
        {
            Content = _cMapControl =  new MapControl();
        }

        ~ControlOnDisposeWithMap00() => Console.WriteLine($"------  Destruct:{GetType().Name}");

        private void Dispose(bool disposing)
        {
            if (disposing)
            {
                _cMapControl.Dispose();
                Content = _cMapControl = null;
            }
        }

        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }
    }
    class ControlWithMapDisposeOnDetach01 : ContentControl
    {
        private MapControl _cMapControl;
        public ControlWithMapDisposeOnDetach01()
        {
            Content = _cMapControl = new MapControl();
        }

        protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e)
        {
            base.OnDetachedFromVisualTree(e);
            _cMapControl?.Dispose();//new idea to test... it run to Destructor
        }

        ~ControlWithMapDisposeOnDetach01() => Console.WriteLine($"------  Destruct:{GetType().Name}");
    }

//Now my code like this one
class ControlWithMapCreateAttach02 : ContentControl
{
private MapControl _cMapControl;

        protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e)
        {
            base.OnAttachedToVisualTree(e);
            Content = _cMapControl = new MapControl();
        }

        protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e)
        {
            base.OnDetachedFromVisualTree(e);
            var temp = _cMapControl;
            Content  = _cMapControl = null;
            temp?.Dispose();
        }

        ~ControlWithMapCreateAttach02() => Console.WriteLine($"------  Destruct:{GetType().Name}");
    }


    private Panel _cPanelRoot;
    public MainWindow()
    {
        InitializeComponent();
        Content = _cPanelRoot = new Panel();

        var timer = new DispatcherTimer(TimeSpan.FromSeconds(5), DispatcherPriority.Render, (sender, args) =>
        {
            var temp = _cPanelRoot.Children.OfType<IDisposable>().ToList();
            _cPanelRoot.Children.Clear();
            temp.ForEach(t => t.Dispose());

            _cPanelRoot.Children.AddRange(new Control[]
            {
                new MapControlEx(),
                new ControlEx(),
                new ControlOnDisposeWithMap00(),
                new ControlWithMapDisposeOnDetach01(),
                new ControlWithMapCreateAttach02(),
            });
            GC.Collect();
        });
    }
}

image

"ControlWithMapDisposeOnDetach01" —— It seems "MapControl" Dispose at first that can run a correct result.
"ControlWithMapDisposeOnDetach01" —— And It is reasonable to explain that there has been no memory leak。

"temp.ForEach(t => t.Dispose());" —— the logic is strange(In principle, i think all instances can execute destructors).

at last i changed the "temp.ForEach(t => t.Dispose());"
image
but result:
image

my logic is a bit confusing. Thank you for your enthusiastic help.

@xiaoliangde
Copy link
Author

If you can provide a minimal reproducible sample I will further investigate this.
“ControlOnDisposeWithMap00”——my earliest implementation method
“ControlWithMapCreateAttach02”——current implementation method
“ControlOnDisposeWithMap01”——The way I just had a sudden idea to add(surprisingly, the result was correct.And It is reasonable to explain that there has been no memory leak)

app.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants