radioman / greatmaps

GMap.NET - Great Maps for Windows Forms & Presentation
912 stars 408 forks source link

crashes when updating lots of markers continously #99

Closed zin33 closed 6 years ago

zin33 commented 6 years ago

hello radioman and thanks a lot for making these awesome libraries :)

my issue is i have a software thats tracking lots of units on the map (may be hundreds) and im updating their position quite often (say every 20 seconds). with only a few markers nothing happens but if i want to show all the markers on the map i get a crash with the red cross

https://stackoverflow.com/questions/24014020/map-control-gmap-net-crashes

i found that thread which seems quite relevant but im not sure howd id go about invoking the markers on the map

anyways ive also found that moving the map around causes the issue to pop much sooner, probably because im forcing the paint method or something. anyways heres the error message

System.InvalidOperationException: collection was modified enumeration operation cannot execute at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource) at System.Collections.Generic.List1.Enumerator.MoveNextRare() at System.Collections.Generic.List1.Enumerator.MoveNext() at GMap.NET.WindowsForms.GMapOverlay.OnRender(Graphics g) at GMap.NET.WindowsForms.GMapControl.OnPaintOverlays(Graphics g) at GMap.NET.WindowsForms.GMapControl.DrawGraphics(Graphics g) at GMap.NET.WindowsForms.GMapControl.OnPaint(PaintEventArgs e) at System.Windows.Forms.Control.PaintWithErrorHandling(PaintEventArgs e, Int16 layer, Boolean disposeEventArgs) at System.Windows.Forms.Control.WmPaint(Message& m) at System.Windows.Forms.Control.WndProc(Message& m) at System.Windows.Forms.ScrollableControl.WndProc(Message& m) at System.Windows.Forms.ContainerControl.WndProc(Message& m) at System.Windows.Forms.UserControl.WndProc(Message& m) at System.Windows.Forms.Control.ControlNativeWindow.OnMessage(Message& m) at System.Windows.Forms.Control.ControlNativeWindow.WndProc(Message& m) at System.Windows.Forms.NativeWindow.Callback(IntPtr hWnd, Int32 msg, IntPtr wparam, IntPtr lparam)

im working on visual studio. vb.net specifically. was using a really old library but i tried with the new one youve got here and sadly i have the same issue.

ive looked at the onRender method and as the thread says its going through all the markers and routes there so if i add one marker or route while its doing so i guess thats when it crashes? maybe adding some synclock to the markers and routes while it does so might fix this? anyways looking forward to your advice! thanks :)

zin33 commented 6 years ago

okay so apparently i fixed it. i inherited the class overlay and added a synclock and that was good enough apparently

leaving the code here for others just in case!


Public Class OverlayOP
    Inherits GMap.NET.WindowsForms.GMapOverlay

    Public Overrides Sub onRender(ByVal g As Graphics)
        If Control Is Nothing Then Exit Sub
        If Control.RoutesEnabled Then
            SyncLock Routes
                For Each r As GMap.NET.WindowsForms.GMapRoute In Routes
                    If r.IsVisible Then r.OnRender(g)
                Next
            End SyncLock
        End If
        If Control.PolygonsEnabled Then
            SyncLock Polygons
                For Each r As GMap.NET.WindowsForms.GMapPolygon In Polygons
                    If r.IsVisible Then r.OnRender(g)
                Next
            End SyncLock
        End If
        SyncLock Markers
            If Control.MarkersEnabled Then ' markers
                For Each m As GMap.NET.WindowsForms.GMapMarker In Markers
                    'if(m.IsVisible && (m.DisableRegionCheck || Control.Core.currentRegion.Contains(m.LocalPosition.X, m.LocalPosition.Y)))
                    If m.IsVisible OrElse m.DisableRegionCheck Then m.OnRender(g)
                Next
                ' tooltips above
                For Each m As GMap.NET.WindowsForms.GMapMarker In Markers
                    'if(m.ToolTip != null && m.IsVisible && Control.Core.currentRegion.Contains(m.LocalPosition.X, m.LocalPosition.Y))
                    If Not m.ToolTip Is Nothing AndAlso m.IsVisible Then
                        If (Not String.IsNullOrEmpty(m.ToolTipText) AndAlso ((m.ToolTipMode = GMap.NET.WindowsForms.MarkerTooltipMode.Always) _
                                    OrElse ((m.ToolTipMode = GMap.NET.WindowsForms.MarkerTooltipMode.OnMouseOver) AndAlso m.IsMouseOver))) Then
                            m.ToolTip.OnRender(g)
                        End If
                    End If
                Next
            End If
        End SyncLock
    End Sub

End Class
tedekeroth commented 5 years ago

I have the exact same issue, Collection was modified. I was looking at the code, and noticed that there is a foreach(GMapMarker m in Markers) and some other collections, but the collections are of type ObservableCollectionThreadSafe which would indicate this shouldnt be a problem, but it is.

Im gonna try to do some locking too, or perhaps create local copies of the lists and iterate over them. MAybe even a for loop instead of a foreach could solve it?

EDIT: I dont understand how a lock around it would help, unless you also use the same lock on all places that modify the collection. Since this is a public property, that seems to be the wrong way to go.

radioman commented 5 years ago

p.s. check https://github.com/radioman/greatmaps/blob/799f3a38a65350ff00c21acf9b88c96848bb3880/Demo.WindowsForms/Forms/MainForm.cs#L432