Opened 10 years ago

Closed 8 years ago

#25961 closed Cleanup/optimization (fixed)

remove handling of thread-non-safe GEOS functions

Reported by: Sergey Fedoseev Owned by: Sergey Fedoseev
Component: GIS Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

All supported GEOS versions provide thread-safe API that means that handling of GEOS functions could be simplified.

Change History (11)

comment:1 by Sergey Fedoseev, 10 years ago

Has patch: set
Owner: changed from nobody to Sergey Fedoseev
Status: new → assigned

PR -- ​https://github.com/django/django/pull/5849

comment:2 by Tim Graham, 10 years ago

Triage Stage: Unreviewed → Accepted

comment:3 by Claude Paroz, 10 years ago

Feel free to go ahead, this is going above my head :-)

in reply to:  3 comment:4 by Sergey Fedoseev, 10 years ago

Replying to claudep:

Feel free to go ahead, this is going above my head :-)

Actually changes in my PR are quite trivial (maybe I did too much in a single commit):

  • GEOSFunc from django/contrib/gis/geos/prototypes/threadsafe.py is simplified because we know that GEOS always have thread-safe API
  • GEOSFunc is merged with GEOSFuncFactory from django/contrib/gis/geos/libgeos.py
  • the rest stuff from django/contrib/gis/geos/prototypes/threadsafe.py is moved in django/contrib/gis/geos/libgeos.py

comment:5 by Tim Graham, 10 years ago

Triage Stage: Accepted → Ready for checkin

comment:6 by Tim Graham <timograham@…>, 10 years ago

Resolution: → fixed
Status: assigned → closed

In 312fc1af:

Fixed #25961 -- Removed handling of thread-non-safe GEOS functions.

comment:7 by Tim Graham, 10 years ago

Has patch: unset
Resolution: fixed
Severity: Normal → Release blocker
Status: closed → new
Triage Stage: Ready for checkin → Accepted

We've seen occasional segmentation faults in the GIS tests on Jenkins after this commit. apollo13 and knbk have been investigating (IRC discussion and on ​a pull request which seems to reproduce the segfault more regularly than on master.

comment:8 by Sergey Fedoseev, 10 years ago

As this change is cleanup only I think it should be reverted until I or someone else will have a good understanding why it's broken and how to fix it.

comment:9 by Tim Graham, 10 years ago

Severity: Release blocker → Normal

The error seems to come from GEOSContextHandle.__del__(). Will revert for now.

comment:10 by Tim Graham, 10 years ago

In 59ef6559a31c175c1f52668ef0ffe9b8d87aa5a5:

Reverted #25961 -- Removed handling of thread-non-safe GEOS functions.

This reverts commit [312fc1a] as it seems
to cause segmentation faults as described in the ticket.

comment:11 by Tim Graham <timograham@…>, 8 years ago

Resolution: → fixed
Status: new → closed

In 8b5a2980:

Fixed #25961 -- Removed handling of non-thread safe GEOS functions.

Note: See TracTickets for help on using tickets.
Back to Top