Working with dynamically loaded layers no longer cause deadlocks & access violations
Several issues were noticed when working with dynamically loaded layers including deadlocks, race conditions and access violations. The dynamic (i.e. extent-based) loading and rendering of large featuresets has been drastically reworked and aligned with how regular feature sets are rendered.
For advanced applications an additional method for OGR layers was added called ExtendFromQuery. This method allows the developer to extend the currently loaded feature set using an OGR SQL query. Features matching the query are added (or updated if a feature with the same OGR_FID already exists) to the feature set.
In previous iterations the ReloadOgrLayerFromSource (on the Map_Layer) was already adapted to allow reloading an entire dynamically loaded feature set.
One day layer and a number of bug reports later from my co-workers I have a few more fixes lined up. I don’t know if Paul already started with the new release - but the fixes are rather important (e.g. deserializing layer styles from a project was broken). They are comitted as I’m typing this.
I just finalized my edits & force updated the git repo with them - sorry if it causes trouble, but I believed this to be cleaner than having another clean up commit.
I have a few more changes locally after these commits which will fix some issues for label categories on dynamic layers but I think it is better to test in depth before releasing it
So , if all seems ok from your end I think we can move forward?
I just pushed a commit that fixed the issue you flagged in step 4.
This compiles & I had a quick run with mapwindow to verify basic functionality which seemed ok. I’ll be testing a little bit more & get back to you & Paul today if I think it’s safe to go forward.
The other changes you made look fine, thanks a bunch for that!
Changes have been committed as follows:
Starting at revision c35bba9, dated Jun 14, 2019, merged older code back into current version, mostly removing the Shapefile-based CriticalSection locking.
I attempted to maintain other, more-recent changes, including qtree updates and initial work on the dynamic OgrLayer. Please review the current code, as I cannot attest to the the current state of the dynamic OgrLayer, other than I believe it is restored to where it was on Jun 14.
I then compared the proposed updates with the previous master release (5.0.1), and considered reverting the OgrLayer back to that revision, but decided, at least for now, to keep it where it was. The primary intent was to revert the Shapefile locking changes that began on May 24 (related to MWGIS-167).
I admit that I may have undone more than was necessary, including the Ogr categories, and ReloadOgrLayerFromSource - but I was unsure where was the best place to make the cutoff.
You may want to now compare with your working copy, and
Perhaps you can confirm with whether you think this would be an adequate patch (understanding that dynamic loading may not be complete, but is otherwise a viable release), and
You can then begin putting back some of the pieces (such as the ReloadOgrLayerFromSource) and we can start tracking down any behavioral issues.
Thanks for the input. I will try to do more review of the dynamic loading process, to get more familiar with it.
Regarding your comment about enforcing single thread access, similar to winforms - this can easily be done by making the Shapefile an Apartment Threaded object. I started to mention this last night, but decided not to get into that just yet. This would accomplish the same thing, and could replace, most of the locking that has been added.
The question is “What is necessary?” This change introduces a fair amount of complexity and overhead that has not been necessary in the OCX up to this point, and I want make sure we only introduce what is necessary, in this case, to accomplish the dynamic loading.
For the time-being, the locking likely doesn’t break anything. But I think it would be good to isolate the issues surrounding the dynamic loading, and reduce the locks to just that. These are just my thoughts. I may try to do some testing with the previous version to see if I can reproduce some of the issues.