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.

Sign up for GitHub

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

Jump to bottom

Fix memory leak in GEOSLineMerge #269

Closed
wants to merge 1 commit into from
Closed

Fix memory leak in GEOSLineMerge #269

wants to merge 1 commit into from

Conversation

dbaston
Copy link
Member

@dbaston dbaston commented Jan 15, 2020

@Algunenano
Copy link
Contributor

I haven't looked at the ramifications around the changes (the move implementation depending on the size doesn't seem necessary in my head, but I don't know the code so I trust Dan), but I can confirm this fixes the leak in GEOSLineMerge we are seeing in Postgis unit tests.

@dbaston
Copy link
Member Author

dbaston commented Jan 16, 2020

Yes, this was a rather lean commit message for so many changes. I'll improve it when I fix the MSVC issue.

the move implementation depending on the size doesn't seem necessary in my head

I think you're referring to buildGeometry ? The semantics of this method are to return the simplest form possible, so for size() == 1 we just return the input and for size() > 1 we construct a collection of the proper type.

@Algunenano
Copy link
Contributor

I think you're referring to buildGeometry ?

Yes, detecting empty, 1 geom, or 1+ geoms seemed excessive, but it makes sense if it's done to use the "lowest" type possible.

@dbaston
Fix memory leak in GEOSLineMerge
6c26d23 
Update LineMerger to return outputs that are managed by unique_ptr.
Update GeometryFactory::buildGeometry to accept inputs of Geometry
subclasses.

Fixes libgeos#1010
@dbaston
Copy link
Member Author

dbaston commented Jan 22, 2020

Looks like I botched the merge. Applied to master as 24f22e7 and 1837d3b

@dbaston dbaston closed this Jan 22, 2020
@pramsey pramsey mentioned this pull request Nov 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@dbaston @Algunenano