girke-lab / ChemmineOB

OpenBabel wrapper package for R
Other
9 stars 5 forks source link

segfault on Ubuntu due to clashing symbols with `grDevices::rgb()` #31

Closed Aariq closed 1 year ago

Aariq commented 1 year ago

This took a while and a lot of help to diagnose, and more details are in this issue: https://github.com/Bioconductor/KEGGREST/issues/19. I'm getting a segfault during devtools::check() in a package that has both ChemmineOB and KEGGREST in Imports. The problem appears to be due to ChemmineOB not "hiding symbols". To reproduce this issue in Ubuntu:

root@abaef95d5e58:~# R_DEFAULT_PACKAGES=NULL R -q -e 'options(warn = 2, showErrorCalls=FALSE); library("ChemmineOB"); grDevices::rgb(1,1,1)'
> options(warn = 2, showErrorCalls=FALSE); library("ChemmineOB"); grDevices::rgb(1,1,1)
Loading required package: methods

 *** caught segfault ***
address 0x7f95244eab00, cause 'invalid permissions'

Traceback:
 1: grDevices::rgb(1, 1, 1)
An irrecoverable exception occurred. R is aborting now ...
Segmentation fault

You can also see an example failing GitHub action in this minimal R package example: https://github.com/Aariq/testpkg/actions/runs/5339333499/jobs/9677910503#step:7:274

Aariq commented 1 year ago

Suggested solution from @gaborcsardi

I think a proper fix is to hide the symbols in ChemmineOB, see 'Writing R Extensions'. Here is an example for how to do that: https://github.com/r-lib/zip/commit/70f0b1518304652ed04a7e7a991396e923030bb7

khoran commented 1 year ago

I can look into this more mid-next week.

khoran commented 1 year ago

I was able to make this work by building OpenBabel with the cmake flag -DENABLE_SYMBOL_VISIBILITY=ON. No modifications to ChemmineOB were required. The 'rgb' symbol is actually in the libopenbabel.so library, not ChemmineOB.so.

Aariq commented 1 year ago

Ok, thanks. Currently the GitHub action I'm using installs OpenBabel on ubuntu runners with apt-get install -y libopenbabel-dev, but it sounds like that's not going to cut it and I'll have to set up the action to compile it, is that correct?

khoran commented 1 year ago

Yea, unfortunately. The conflicting symbol is in that library, so there is no getting around it.

Aariq commented 1 year ago

Do you happen to know why getNativeSymbolInfo("rgb") points to ChemmineOB.so if rgb is in libopenbabel.so?

> getNativeSymbolInfo("rgb")
$name
[1] "rgb"

$address
<pointer: 0x11417c510>
attr(,"class")
[1] "NativeSymbol"

$dll
DLL name: ChemmineOB
Filename:
          /Library/Frameworks/R.framework/Versions/4.2/Resources/library/ChemmineOB/libs/ChemmineOB.so
Dynamic lookup: TRUE

attr(,"class")
khoran commented 1 year ago

I'm guessing because it links to libopenbabel.so. It is the entry point from R to the openbabel library.

khoran commented 1 year ago

if you run nm on libopenbabel.so and search for rgb you'll see it.

gaborcsardi commented 1 year ago

@Aariq That's unfortunate, and I am afraid that the only workaround (that I know) is to import the rgb() function from the grDevices package.

That's a free import because grDevices is almost always loaded automatically up front anyway:

❯ R --vanilla -q -e 'loadedNamespaces()'
> loadedNamespaces()
[1] "compiler"  "graphics"  "utils"     "grDevices" "stats"     "datasets"
[7] "methods"   "base"

But in R CMD check sometimes it is not, and that makes packages depending on ChemmineOB crash.