#2412 closed defect (fixed)
fix memory leaks
| Reported by: | tomkralidis | Owned by: | tomkralidis |
|---|---|---|---|
| Priority: | normal | Milestone: | 5.2 release |
| Component: | SOS Server | Version: | svn-trunk (development) |
| Severity: | normal | Keywords: | sos |
| Cc: | assefa, dmorissette, sdlime |
Description
There are various memory leaks / issues when running msautotest/wxs/run_tests.py -valgrind ./msautotest/wxs/sos.map, which can be displayed when checking out the output in msautotest/wxs/results/sos_*.xml.vgrind.txt.txt.
Change History (8)
comment:1 by , 16 years ago
follow-up: 4 comment:2 by , 16 years ago
| Cc: | added |
|---|---|
| Owner: | changed from to |
Tom, are you sure that r7098 is the correct fix? By removing the strdup() calls you actually make references in the sosParamsObj to memory owned by the cgiRequestObj... whether this is a good or a bad thing is arguable, but that makes me nervous in this specific case.
In addition to that, msSOSFreeParamsObj() will attempt to free the memory pointed to by sosparams->pszVersion and other members... but this memory is owned by the cgiRequestObj so it's bad bad bad to do that.
To be consistent with the way we do things elsewhere in MapServer, specifically the wfsParams data structure from which you probably copied the idea, I think you should revert your changes to strdup() all param names as before, and fix the leak by calling msSOSFreeParamsObj() everywhere it is needed.
comment:3 by , 16 years ago
| Cc: | added |
|---|
follow-up: 5 comment:4 by , 16 years ago
Replying to dmorissette:
Tom, are you sure that r7098 is the correct fix? By removing the strdup() calls you actually make references in the sosParamsObj to memory owned by the cgiRequestObj... whether this is a good or a bad thing is arguable, but that makes me nervous in this specific case.
In addition to that, msSOSFreeParamsObj() will attempt to free the memory pointed to by sosparams->pszVersion and other members... but this memory is owned by the cgiRequestObj so it's bad bad bad to do that.
To be consistent with the way we do things elsewhere in MapServer, specifically the wfsParams data structure from which you probably copied the idea, I think you should revert your changes to strdup() all param names as before, and fix the leak by calling msSOSFreeParamsObj() everywhere it is needed.
Daniel: I'm confused; msSOSParseRequest in latest svn trunk does strdup() all param names. r7098 adds the strdup() calls, not remove them.
comment:5 by , 16 years ago
Replying to tomkralidis:
Daniel: I'm confused; msSOSParseRequest in latest svn trunk does strdup() all param names. r7098 adds the strdup() calls, not remove them.
Oopps... sorry, I got into this ticket thinking it was about memory leaks only (as the ticket summary suggests) and didn't notice that the strdup() were actually added and not removed (i.e. fixing a double free instead of a leak).
I think attaching the valgrind output to the ticket, combined with a more complete description of the fixes in comment:1 would have helped... that's why I had to go view the changeset to figure what this "first pass of changes in r7098" comment meant exactly.
comment:6 by , 16 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
Fixed a few of the more major leaks which came with the initial SOS 1.0.0 efforts. Closing for now. Can be reopened as required or new tickets against specific leaks.
FYI first pass of changes in r7098