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

Memory leak #82

Closed
fergusreiggracia opened this issue Jan 13, 2021 · 34 comments
Closed

Memory leak #82

fergusreiggracia opened this issue Jan 13, 2021 · 34 comments

Comments

@fergusreiggracia
Copy link

My program had a memory loss loop. Each execution needed more and although deleting all the objects of R and calling gc () it did not recover it, looking for the error I have reached a loop only with a call to gstat.cv that "loses" memory in each execution until it left the machine no memory (has 64GB).

I am using gstat_2.0-6 which I have installed with Conda.

I attach the data with which I call the function
cv <- gstat.cv(dat.uk.mod, verbose = FALSE, debug.level = 0)
dat.uk.mod.zip
@edzer
Copy link
Member

edzer commented Jan 14, 2021

Thanks, I can reproduce this. @rsbivand : a report obtained with R --vanilla -d 'valgrind --leak-check=full' pointed to "posssibly lost" leaks in rgdal, mainly occuring in rgdal (proj6.cpp:941, ogr_proj.cpp:169, ogr_proj.cpp:174). Let me know if you want me to look further into this, and/or share the full valgrind report.

@rsbivand
Copy link
Member

rsbivand commented Jan 14, 2021 •
edited
Loading

@edzer Please follow up to the point at which there is an rgdal only reprex. Is the problem that predict.gstat() calls sp::proj4string() repeatedly? Also, not reproducible for me, command runs to completion with no memory consequences (flat usage).

@fergusreiggracia
Copy link
Author

R version is 3.6.3 on Linux

while(TRUE){
cv <- gstat.cv(dat.uk.mod, verbose = FALSE, debug.level = 0)
}
64GB of RAM overflows and if all the objects in the session are deleted, nothing is recovered, just closing R.

@rsbivand
Copy link
Member

@MuDestructor No, the full details are needed. Package versions are much more important than R version, and for sf and sp/rgdal, the output of sf::sf_extSoftVersion() and rgdal::rgdal_extSoftVersion(), and since you probably installed these packages from source building agaist these exxternal libraries, the full details of how you installed those libraries. In my case:

 > packageVersion("rgdal")
[1] ‘1.5.19’
> rgdal::rgdal_extSoftVersion()
          GDAL GDAL_with_GEOS           PROJ             sp 
       "3.2.1"         "TRUE"        "7.2.1"        "1.4-4" 
> packageVersion("sp")
[1] ‘1.4.5’

PROJ and GDAL installed from source.

@fergusreiggracia
Copy link
Author

fergusreiggracia commented Jan 14, 2021 •
edited
Loading

Installed with Conda on a Debian machine

packageVersion("rgdal")
[1] ‘1.5.18’
packageVersion("sp")
[1] ‘1.4.2’
packageVersion("gstat")
[1] ‘2.0.6

And same result on an Ubuntu machine, without Conda, installed from source.

packageVersion("rgdal")
[1] ‘1.5.19’
packageVersion("sp")
[1] ‘1.4.5’
packageVersion("gstat")
[1] ‘2.0.6’

@rsbivand
Copy link
Member

No, I really need the output of rgdal::rgdal_extSoftVersion() as I said. rgdal links to PROJ and GDAL directly. I suspect that your system has a non-current PROJ (at least), and that any memory leakage has been patched in later releases. You haven't said what OS or version you are using. Good that you dropped conda, it always makes debugging harder. Are you using R in the console or behind RStudio?

@edzer
Copy link
Member

edzer commented Jan 14, 2021

The leaks I saw point to rgdal 1.5-19, linking to GDAL 3.2.0, PROJ 7.2.0, sp 1.4-4. Will try to isolate the rgdal calls.

@rsbivand
Copy link
Member

And which PROJ installation path? Is it a specific binary package?

@edzer
Copy link
Member

edzer commented Jan 14, 2021

source install, PROJ dynamically linked, ubuntu 20.04, using ubuntugis-unstable for focal.

@rsbivand
Copy link
Member

But did your memory max. out in this command? Mine did not, the memory resource was flat. Or did you just see apparent leaks from valgrind running that command?

@edzer
Copy link
Member

edzer commented Jan 14, 2021

I just saw the leaks, and see a linear memory increase when running in the while(TRUE) { } loop.

@rsbivand
Copy link
Member

while(TRUE) {} is not a use case. Were there leaks from a single run, if so, where? And why is gstat still using sp::proj4string(), leaving the output prediction with no CRS? What did you do to run valgrind, did you make the reprex into a script?

@edzer
Copy link
Member

edzer commented Jan 14, 2021

Running the single command also caused memory leaks. The script I ran was

library(gstat)
dat.uk.mod = readRDS("dat.uk.mod.rds")
cv <- gstat.cv(dat.uk.mod, verbose = FALSE, debug.level = 0)

(using the data provided above), the command I ran was

R --no-restore --no-save -q -d 'valgrind  --leak-check=full' < xx > out

and I didn't yet come to the conclusion that sp::proj4string is causing this. I'm ATM slowed down by other obligations.

@rsbivand
Copy link
Member

rsbivand commented Jan 14, 2021 •
edited
Loading

Running debug() on gstat.cv() called sp::proj4string() which may well precipitate attempts to rebuild the CRS. The problem is in line 941 of rgdal/src/proj6.cpp, in P6_SRID_proj(). I'm re-trying with PJ_DEFAULT_CTX instead of a locally created context, but I'm pretty certain that source_crs is being destroyed correctly. After running, the same leak is present, I didn't commit to R-Forge. CRAN doesn't show any valgrind issues for rgdal. There is an additional loss in OSR_is_projected() in rgdal/src/ogr_proj.cpp, I'll try that too.

Should sp be caching spurious and repeated proj4string() calls?

@fergusreiggracia
Copy link
Author

I really appreciate the help from @rsbivand and @edzer I think @edzer has already answered all the doubts, some I don't understand well because of my knowledge of R or the language.

Of course, a while (TRUE) loop is not a use case, but it was leaking memory in an unknown and unrecoverable way and came up to me as a way to locate the leak.

Doing
o <- new ("Spatial")
while (TRUE) {
proj4string (o) <- CRS ("+ init = epsg: 27700")
}
Memory is lost ... but when doing
rm (list = ls ())
gc ()
The memory is freed.

What I can't do when I lose her with
cv <- gstat.cv (dat.uk.mod, verbose = FALSE, debug.level = 0)

@rsbivand
Copy link
Member

I cannot see any memory leakage with proj4string<-(). What was the original data object used as input when dat.uk.mod was created, is uk universal kriging or United Kingdom? The CRS of cat(wkt(dat.uk.mod$data$var1$data), "\n") isn't EPSG:27700, and at that point is already mangled (lost datum but has a WKT without a datum). I'd like to track the steps leading to the creation of this object.

@fergusreiggracia
Copy link
Author

uk is universal kriging.

I attach data from the statement that creates the object
dat.uk.mod <- gstat(formula = kridge.vars, data = dat_sp, model = v.fit, nmax = n.max, weights = errors)

rds.zip

This is the CRS
crs : +proj=utm +zone=30 +ellps=intl +units=m +no_defs

I don't want to waste your time, I just wanted to help solve what I thought was a possible problem.

@rsbivand
Copy link
Member

rsbivand commented Jan 14, 2021 •
edited
Loading

Please try remotes::install_github("rsbivand/gstat"); I edited the few proj4string instances to try to stop them going out to rgdal too many times to confirm the CRS. On my machine the major leak does not appear with this fix.

@edzer
Copy link
Member

edzer commented Jan 14, 2021 •
edited
Loading

Great! That strongly reduces the memory leaks. These remain:

==235760== Memcheck, a memory error detector
==235760== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==235760== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==235760== Command: /usr/lib/R/bin/exec/R --no-restore --no-save -q
==235760== 
> library(gstat)
> dat.uk.mod = readRDS("dat.uk.mod.rds")
> cv <- gstat.cv(dat.uk.mod, verbose = FALSE, debug.level = 0)
There were 50 or more warnings (use warnings() to see the first 50)
> 
> 
==235760== 
==235760== HEAP SUMMARY:
==235760==     in use at exit: 134,297,103 bytes in 44,621 blocks
==235760==   total heap usage: 1,473,841 allocs, 1,429,220 frees, 907,051,010 bytes allocated
==235760== 
==235760== 600 bytes in 50 blocks are possibly lost in loss record 3,991 of 6,375
==235760==    at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==235760==    by 0x109D5CAC: OGRSpatialReference::Private::Private() (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x109DF510: OGRSpatialReference::OGRSpatialReference(char const*) (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x105B9D84: OSR_is_projected (ogr_proj.cpp:169)
==235760==    by 0x493FB7F: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x49400B5: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x497A1D0: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4996C9E: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4997B91: Rf_applyClosure (in /usr/lib/R/lib/libR.so)
==235760==    by 0x498450D: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760== 
==235760== 800 bytes in 50 blocks are possibly lost in loss record 4,023 of 6,375
==235760==    at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==235760==    by 0x109D5C01: OGRSpatialReference::Private::Private() (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x109DF510: OGRSpatialReference::OGRSpatialReference(char const*) (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x105B9D84: OSR_is_projected (ogr_proj.cpp:169)
==235760==    by 0x493FB7F: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x49400B5: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x497A1D0: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4996C9E: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4997B91: Rf_applyClosure (in /usr/lib/R/lib/libR.so)
==235760==    by 0x498450D: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760== 
==235760== 808 bytes in 1 blocks are possibly lost in loss record 4,025 of 6,375
==235760==    at 0x483C0F3: operator new(unsigned long, std::nothrow_t const&) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==235760==    by 0x11E237B8: ??? (in /usr/lib/x86_64-linux-gnu/libproj.so.19.2.0)
==235760==    by 0x11DAFB04: ??? (in /usr/lib/x86_64-linux-gnu/libproj.so.19.2.0)
==235760==    by 0x11DBDAE3: proj_create_from_wkt (in /usr/lib/x86_64-linux-gnu/libproj.so.19.2.0)
==235760==    by 0x109DF300: OGRSpatialReference::importFromWkt(char const**) (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x109DF4D6: OGRSpatialReference::importFromWkt(char const*) (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x109E49A2: OGRSpatialReference::SetFromUserInput(char const*) (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x105B9DA6: OSR_is_projected (ogr_proj.cpp:174)
==235760==    by 0x493FB7F: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x49400B5: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x497A1D0: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760== 
==235760== 1,200 bytes in 50 blocks are possibly lost in loss record 4,053 of 6,375
==235760==    at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==235760==    by 0x109D5C33: OGRSpatialReference::Private::Private() (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x109DF510: OGRSpatialReference::OGRSpatialReference(char const*) (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x105B9D84: OSR_is_projected (ogr_proj.cpp:169)
==235760==    by 0x493FB7F: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x49400B5: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x497A1D0: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4996C9E: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4997B91: Rf_applyClosure (in /usr/lib/R/lib/libR.so)
==235760==    by 0x498450D: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760== 
==235760== 24,800 bytes in 50 blocks are possibly lost in loss record 5,731 of 6,375
==235760==    at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==235760==    by 0x109DF505: OGRSpatialReference::OGRSpatialReference(char const*) (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x105B9D84: OSR_is_projected (ogr_proj.cpp:169)
==235760==    by 0x493FB7F: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x49400B5: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x497A1D0: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4996C9E: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4997B91: Rf_applyClosure (in /usr/lib/R/lib/libR.so)
==235760==    by 0x498450D: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4996C9E: ??? (in /usr/lib/R/lib/libR.so)
==235760== 
==235760== 94,536 bytes in 117 blocks are possibly lost in loss record 6,181 of 6,375
==235760==    at 0x483C0F3: operator new(unsigned long, std::nothrow_t const&) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==235760==    by 0x11E237B8: ??? (in /usr/lib/x86_64-linux-gnu/libproj.so.19.2.0)
==235760==    by 0x11DAFB04: ??? (in /usr/lib/x86_64-linux-gnu/libproj.so.19.2.0)
==235760==    by 0x11DAFD13: proj_clone (in /usr/lib/x86_64-linux-gnu/libproj.so.19.2.0)
==235760==    by 0x1097EB74: ??? (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x109DF1C3: OGRSpatialReference::importFromWkt(char const**) (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x109DF4D6: OGRSpatialReference::importFromWkt(char const*) (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x109E49A2: OGRSpatialReference::SetFromUserInput(char const*) (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x105B9DA6: OSR_is_projected (ogr_proj.cpp:174)
==235760==    by 0x493FB7F: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x49400B5: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x497A1D0: ??? (in /usr/lib/R/lib/libR.so)
==235760== 
==235760== 235,712 (2,848 direct, 232,864 indirect) bytes in 178 blocks are definitely lost in loss record 6,317 of 6,375
==235760==    at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==235760==    by 0x109D5C01: OGRSpatialReference::Private::Private() (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x109DF510: OGRSpatialReference::OGRSpatialReference(char const*) (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x105B9D84: OSR_is_projected (ogr_proj.cpp:169)
==235760==    by 0x493FB7F: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x49400B5: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x497A1D0: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4996C9E: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4997B91: Rf_applyClosure (in /usr/lib/R/lib/libR.so)
==235760==    by 0x498450D: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760== 
==235760== 1,098,732 (19,704 direct, 1,079,028 indirect) bytes in 821 blocks are definitely lost in loss record 6,359 of 6,375
==235760==    at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==235760==    by 0x109D5C33: OGRSpatialReference::Private::Private() (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x109DF510: OGRSpatialReference::OGRSpatialReference(char const*) (in /usr/lib/libgdal.so.28.0.0)
==235760==    by 0x105B9D84: OSR_is_projected (ogr_proj.cpp:169)
==235760==    by 0x493FB7F: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x49400B5: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x497A1D0: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4996C9E: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4997B91: Rf_applyClosure (in /usr/lib/R/lib/libR.so)
==235760==    by 0x498450D: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760== 
==235760== 1,859,520 (38,720 direct, 1,820,800 indirect) bytes in 2,420 blocks are definitely lost in loss record 6,365 of 6,375
==235760==    at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==235760==    by 0x105B9D77: OSR_is_projected (ogr_proj.cpp:169)
==235760==    by 0x493FB7F: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x49400B5: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x497A1D0: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4996C9E: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4997B91: Rf_applyClosure (in /usr/lib/R/lib/libR.so)
==235760==    by 0x498450D: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4994DD7: Rf_eval (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4996C9E: ??? (in /usr/lib/R/lib/libR.so)
==235760==    by 0x4997FF8: R_execMethod (in /usr/lib/R/lib/libR.so)
==235760== 
==235760== LEAK SUMMARY:
==235760==    definitely lost: 61,272 bytes in 3,419 blocks
==235760==    indirectly lost: 3,132,692 bytes in 10,771 blocks
==235760==      possibly lost: 122,744 bytes in 318 blocks
==235760==    still reachable: 130,980,395 bytes in 30,113 blocks
==235760==                       of which reachable via heuristic:
==235760==                         newarray           : 4,264 bytes in 1 blocks
==235760==         suppressed: 0 bytes in 0 blocks
==235760== Reachable blocks (those to which a pointer was found) are not shown.
==235760== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==235760== 
==235760== For lists of detected and suppressed errors, rerun with: -s
==235760== ERROR SUMMARY: 9 errors from 9 contexts (suppressed: 0 from 0)

@rsbivand
Copy link
Member

rsbivand commented Jan 14, 2021 via email

@edzer
Copy link
Member

edzer commented Jan 14, 2021

Hmmm.

@rsbivand
Copy link
Member

Gone with rev. 1093, current build state. I'm unsure why CRS(projstring(.)) might cycle, or why proj_create() might leak. Should I make a PR for gstat?

@fergusreiggracia
Copy link
Author

Memory loss has decreased. Thanks.

@rsbivand
Copy link
Member

Does installing install.packages("rgdal", repos="http://R-Forge.R-project.org") once the build is confirmed on https://r-forge.r-project.org/R/?group_id=884 remove more? One problem was gstat calling sp::CRS() when it didn't need to, and sp::proj4string() and its assignment method too. Another was that calls to check that the CRS is planar did hit a leak in code calling GDAL in rgdal, now fixed.

@fergusreiggracia
Copy link
Author

Yes, that way you lose even less memory. Will those changes go to the package in a while?

@edzer edzer mentioned this issue Jan 15, 2021
Merged
@edzer
Copy link
Member

edzer commented Jan 15, 2021

Indeed, with rgdal rev 1094 and your gstat fork, memory leaks are gone for this use case. I merged your gstat fork into master, thanks! Running this with rgdal rev 1094 and gstat from CRAN still leaks memory, so there is still something to hunt down I believe, but this case is solved.

@edzer edzer closed this as completed Jan 15, 2021
@rsbivand
Copy link
Member

The leaks I see are from RGDAL_Init(), which registers drivers. I added

  OGRCleanupAll();
  OSRCleanup();
  GDALDestroyDriverManager();

to RGDAL_Exit() in 1094, but the leak is still there, there is something in .onUnload() I don't understand. Does sf leak in the same way?

@rsbivand
Copy link
Member

rsbivand commented Jan 18, 2021 •
edited
Loading

@edzer Is it possible that cases like:

// valgrind 210115
if ((source_crs = proj_create(PJ_DEFAULT_CTX, CHAR(STRING_ELT(inSRID, 0)))) == NULL) 
if ((pj_transform = proj_create_crs_to_crs_from_pj(PJ_DEFAULT_CTX, 
            source_crs, target_crs, area_of_interest, NULL)) == 0)

    papszOptions = CSLAddString(papszOptions, "FORMAT=WKT2_2018");
// valgrind 210115
    papszOptions = CSLAddString(papszOptions, "MULTILINE=YES");

// valgrind 210115
        if (hSRS->exportToWkt(&wkt2, papszOptions) != OGRERR_NONE)

                OGRPolygon** papoPolygons = new OGRPolygon*[ Lns_l ];
                for (k=0; k<Lns_l; k++) {
// valgrind 210115
                    papoPolygons[k] = new OGRPolygon();
// valgrind 210115
                    OGRLinearRing *OGRlr = new OGRLinearRing;
// valgrind 210115
                        OGRlr->setPoint( j, NUMERIC_POINTER(crds)[j],
                                    NUMERIC_POINTER(crds)[j+ncrds] );

are related to PROJ and GDAL now being C++11, but rgdal not being (or requiring) C++11?

@rsbivand
Copy link
Member

No, not C++11, already taken into account.

@edzer
Copy link
Member

edzer commented Jan 19, 2021

In proj6.cpp, line 958 and further, if I do

    if (vis_order) {
        PJ *orig_crs = source_crs;
        source_crs = proj_normalize_for_visualization(PJ_DEFAULT_CTX, orig_crs);
        proj_destroy(orig_crs);

the memory leak is gone, for me, with older gstat and newest (svn) rgdal. Haven't committed this.

@rsbivand
Copy link
Member

rsbivand commented Jan 20, 2021 •
edited
Loading

Thanks, this is one of a dozen or more leaks in 4 files, proj6, gdal-bindings, ogr_proj and OGR_write(.cpp). This is annotated // valgrind 210115, the remainder // valgrind 210119. Committed as rev 1095. Who might help with the remaining ones?

It looks as though my lack of knowledge of newer C++ is biting, and I cannot grasp why the objects don't behave like C structs (I think). I would never have expected there to be a problem having the same object pointer on both sides of an assignment, nor that C++ would create a new object that then didn't get cleaned up.

@edzer
Copy link
Member

edzer commented Jan 20, 2021

It is not so much a C or C++ thing but more a style of programming (return a newly allocated object rather than modify the one passed) that needs getting used to. It's documented both in the tutorial and the docs.

@rsbivand
Copy link
Member

rsbivand commented Jan 20, 2021 •
edited
Loading

Does this also apply to GDAL (about half of the remaining leaks are GDAL)? Thanks for the links!

All leaks in proj6.cpp gone in rev. 1096, all were about proj_normalize_for_visualization().

@rsbivand
Copy link
Member

rsbivand commented Jan 21, 2021 •
edited
Loading

Spoken too soon. project_ng() has leaks (seen in rgdal-Ex.Rout), flagged at line 895 in rev 1097. No idea what is wrong - does pj_transform need creating for each coordinate object? Yes, the leaks are not reported (so presumably gone) when the transformation object is created and destroyed in each iteration, unfortunately.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Sep 20, 2021
(geography/R-rgdal) Updated 1.4.8 to 1.5.27
aacc808 
# Please note that **rgdal** will be  retired by the end of 2023, plan
  transition to sf/stars/terra  functions using GDAL and  PROJ at your
  earliest convenience.

# Version 1.5-27 (development, rev. 1149-)
* Correcting logic error in check for MXE UCRT builds (temporary)

# Version 1.5-26 (2021-09-15, rev. 1141-1148)
* Mute use of PROJ CDN for MXE UCRT builds (temporary)

* Run autoupdate on configure.ac to handle obsolete AC_HELP_STRING etc.

# Version 1.5-25 (2021-09-08, rev. 1122-1140)
* Add environment variable access to --with-data-copy by
  PROJ_GDAL_DATA_COPY (r-spatial/sf#1605)

* Adaptations for PROJ 8.

* Handle mixed 2D/3D in readOGR():
  (r-spatial/sf#1683)

* Add tweak for UCRT builds.

* Thin examples for spTransform().

# Version 1.5-23 (2021-02-03, rev. 1120-1121)
* Further fallout after removing valgrind issues.

# Version 1.5-22 (2021-02-02, rev. 1106-1119)
* Attempt to remove further valgrind leak in proj6.cpp: PROJcopyEPSG()
  and in ogr_proj.cpp, both wrongly placed object destructors.

* Modified roundtripping all declared projections in ?project examples
  because some listed projections for PROJ >= 5 provoke valgrind
  leakages by returning very large out-of-scope values for input
  coordinates (0, 0); inversion of these is not attempted; some listed
  projections are not projections.

# Version 1.5-21 (2021-01-27, rev. 1093-1105)
* Suggest **rgeos** to write pre-SFS multipolygon objects to avoid
  unpleasant workaround.

* Try to eliminate current valgrind leaks, starting from
  (r-spatial/gstat#82).

* Try to increase robustness to installation with early PROJ 6
  versions, which often lack functionality found necessary later (for
  example visualization order); the code had assumed that this
  function always was available and behaved as it now does. There are
  now graceful failures when not available.

# Version 1.5-19 (2021-01-05, rev. 1083-1092)
* Dan Baston: raster speedups

* PROJ 7.2.1 includes a bug-fix for `+proj=ob_tran` cases that
  required changes in detection and handling of target/source CRS
  reversal,
  https://lists.osgeo.org/pipermail/proj/2020-December/009999.html

# Version 1.5-18 (2020-10-13, rev. 1071-1082)
* condition `tests/test_enforce_xy.R` on PROJ >= 6 and GDAL >= 3
  (email BDR, I forgot to re-check with PROJ-5.2.0/GDAL-2.2.4).

* Adaptation to EPSG 10 database format started (from PROJ 7.2);
  choose DB columns by name not number in vignette.

# Version 1.5-17 (2020-10-08, rev. 1051-1070)
* `"CRS"` instantiation now prefers PROJ: use
  `rgdal::set_prefer_proj(FALSE)` to return to earlier behaviour. It
  seems that access from C/C++ code to mechanisms in PROJ offers more
  depth than going through GDAL to PROJ. This `"CRS"` instantiation in
  **sp** and **raster**; Proj4 and WKT2 strings may differ depending
  on whether instantiation is straight from PROJ or goes via
  GDAL. Confirmed with multiple reverse dependency checks over almost
  900 CRAN packages.

* By default use PROJ function to extract the source CRS from a
  `"BOUNDCRS"`. When `+towgs84=` is given, PROJ and GDAL see the
  apparent source Proj4 string as implicitly implying a coordinate
  operation transforming to target WGS84, leading to the WKT2
  representation being a `"BOUNDCRS"`, not a `"PROJCRS"` or
  `"GEOGCRS"`, and thus causing misunderstandings later in searching
  for the most accurate coordinate operation for a transformation. May
  be controlled by setting the `get_source_if_boundcrs=` in
  `sp::CRS()` from **sp** 1.4-4 (2020-10-07). Confirmed with multiple
  reverse dependency checks over almost 900 CRAN packages.

* Add support for instantiating from `"OGC:CRS84"` to provide a
  guaranteed GIS/visualization axis order WGS84 instantiator
  (preferred to `"EPSG:4326"`).

* Permit empty string in `SRS_string=` argument to `sp::CRS()` and
  functions called by it.

* Use GDAL `ORSIsProjected()` instead of simply looking for
  `"+proj=longlat"` in the Proj4 string representation where possible.

# Version 1.5-16 (2020-08-07, rev. 1047-1050)
* Typo in C code; use `try()` around Area-of-Interest calculation for
  coordinate operations (email BDR, I forgot to re-check with
  PROJ-5.2.0/GDAL-2.2.4).

# Version 1.5-15 (2020-08-04, rev. 1020-1046)
* Add support for instantiating from `"ESRI:"`.

* Add Area-of-Interest to coordinate operation search (reduces the
  number of candidates found in many cases), and use in
  `rgdal::spTransform()` by default (`use_aoi=FALSE` to suppress);
  illustrate in vignette
  https://cran.r-project.org/web/packages/rgdal/vignettes/CRS_projections_transformations.html.

* Harden to condition on PROJ functions only available from 6.2.0;
  block `"+proj=ob_tran` tests for PROJ 6.0.0-6.1.1.

* Support PROJ CDN https://cdn.proj.org for on-demand download of
  transformation grids if requested by user; document in vignette
  https://cran.r-project.org/web/packages/rgdal/vignettes/CRS_projections_transformations.html.

# Version 1.5-12 (2020-06-26, rev. 1007-1019)
* Further corrections to `configure.ac` for older PROJ/GDAL versions

# Version 1.5-10 (2020-06-09, rev. 991-1006)
* Corrections to `configure.ac` for older PROJ/GDAL versions

# Version 1.5-8 (2020-05-28, rev. 846-990)
* Released to match **sp** 1.4.0 (2020-02-21) to 1.4-2 (2020-05-20)
  following months of development adapting to breaking changes in the
  external libraries used here: PROJ and GDAL; see also
  https://cran.r-project.org/web/packages/sp/news.html.

* Expose `options("rgdal_show_exportToProj4_warnings"="none")` to mute
  Proj4 string degradation warnings.

* Add new vignette
  https://cran.r-project.org/web/packages/rgdal/vignettes/CRS_projections_transformations.html.

* CRAN Windows binary uses PROJ >= 6 and GDAL >= 3

* Add PROJ-based CRS comparison: `compare_CRS()`.

* `project()` and `spTransform()` use WKT2 comment if available,
  fallback to Proj4 representation if not.

* List coordinate operations (based on pyproj code): `list_coordOps()`.

* Add `enforce_xy=` arguments to try to ensure that only
  GIS/visualization axis order is present.

* Add `"CRS"` object comment carrying WKT2 (2019) multiline string
  representation on read operations.

* Use `"CRS"` object comment carrying WKT2 (2019) multiline string
  representation on write operations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@edzer @fergusreiggracia @rsbivand