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.

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

Importing fst into another package causes the other package to crash R #109

Closed
eipi10 opened this issue Dec 1, 2017 · 26 comments
Closed
Assignees
Labels
Milestone

Comments

@eipi10
Copy link

eipi10 commented Dec 1, 2017

I've been building a couple of packages to load various personal utility functions. While building and using the packages, I was having problems where R would crash at some point during the package updating process (devtools::document(), devtools::build(), etc.), but after the crash I would be able to get one step farther in the updating process before a crash. But then after installing the package, R would crash either when I loaded the package or when I ran a function from the package. Or, sometimes, the package wouldn't crash, but even after loading it, R wouldn't recognize any functions from the package when I tried to run them.

After some experimentation, it looks like importing fst is what is causing the crashes. Or, at least, not importing fst stopped the crashes from happening. I can even use fst::read.fst(...) inside functions in the package and avoid a crash, as long as I don't explicitly import fst.

I was able to replicate the crashing behavior by opening a new R Package project in RStudio. In this empty project I created one more file called imports.r containing the following code:

#' @import fst
NULL

After saving this file in the R folder and running devtools::document(), RStudio crashes. If I open a regular R session and run devtools::document() R crashes. If I reopen the project in either R or RStudio I can run devtools::document() without a crash, but then devtools::build() results in a crash (or sometimes I can build, but then devtools::install() results in a crash).

If I change #' @import fst to #' @import dplyr (or any other package I've tried) then the crashing stops.

I'm not sure why this problem is occurring, but I'd be happy to provide additional information. Just let me know. Below is the description file for my version of fst (this is a development version that I updated to a couple of weeks ago) and some details about my system. Also, I asked a question about this at StackOverflow 10 days ago, but haven't gotten any replies. I'll post an answer there soon about the cause and (hopefully) a resolution.

Macbook Pro running OSX Sierra 10.12.6
R: 3.4.2
RStudio: 1.1.392
devtools: 1.13.4

Package: fst
Type: Package
Title: Lightning Fast Serialization of Data Frames for R
Description: Read and write data frames at high speed. Compress your data with fast and efficient type-optimized algorithms that allow for random access of stored data frames (columns and rows).
Version: 0.7.3
Date: 2017-01-11
Authors@R: c(
    person("Mark", "Klik", ,"markklik@gmail.com", role = c("aut", "cre", "cph")))
LazyData: true
Depends: R (>= 3.0.0)
Imports: Rcpp
LinkingTo: Rcpp
SystemRequirements: little-endian platform
RoxygenNote: 6.0.1
Suggests: testthat, bit64, data.table, lintr, nanotime
License: AGPL-3 | file LICENSE
Copyright: This package includes sources from the LZ4 library written
        by Yann Collet, sources of the ZSTD library owned by Facebook,
        Inc. and sources of the fstlib library owned by Mark AJ Klik
URL: https://fstpackage.github.io
BugReports: https://github.com/fstpackage/fst/issues
Author: Mark Klik [aut, cre, cph]
Maintainer: Mark Klik <markklik@gmail.com>
Built: R 3.4.2; x86_64-apple-darwin15.6.0; 2017-11-21 20:23:45 UTC; unix
RemoteType: github
RemoteHost: https://api.github.com
RemoteRepo: fst
RemoteUsername: fstPackage
RemoteRef: develop
RemoteSha: 6f240c5ddbde62b7d97af1157473ba923dca912b
GithubRepo: fst
GithubUsername: fstPackage
GithubRef: develop
GithubSHA1: 6f240c5ddbde62b7d97af1157473ba923dca912b
@MarcusKlik
Copy link
Collaborator

Hi @eipi10, thanks for reporting your issue! That's a curious problem indeed. Because you're using OSX Sierra, I'm thinking that the OpenMP multi-threading might be the cause of the problem. The data.table package has a similar OpenMP implementation and some issues were reported with compiling on OSX (Sierra).

It would be great if you could try building the empty RStudio project with an import of data.table instead of fst, just to check if the OpenMP might be the cause.
Also manually removing fst before compiling the (latest) dev version might help with compiler issues, some issues with mixed versions were reported before.

Package heims imports fst and I can build it without problems on Windows and Ubuntu. The same holds for the rio package but it only suggests fst, so in that case your issues wouldn't apply.

I'll try to reproduce your empty project crash myself although I don't have access to an OSX Sierra box currently (if that's the cause).

Thanks for reporting (and testing)!

@MarcusKlik MarcusKlik self-assigned this Dec 2, 2017
@MarcusKlik MarcusKlik added the bug label Dec 2, 2017
@MarcusKlik
Copy link
Collaborator

MarcusKlik commented Dec 2, 2017

Hi @eipi10, I've build an empty RStudio package that only imports fst and has a single method that calls methods from fst (public and internal).

Here is the zipped package directory (hope you can unpack that cleanly).

I followed the following steps to create, build and check it:

  1. remove fst package (via RStudio interface)
  2. build a fst source package (are you seeing the -fopenmp compiler flags?)
  3. install the fst source package created in step 2 (32-bit and 64-bit version should be build)
  4. create new RStudio project fsttester (non Rcpp)
  5. add new file import.R as specified in your issue
  6. add fst to imports field in DESCRIPTION
  7. remove NAMESPACE file (otherwise roxygen2 won't overwrite)
  8. remove all files in man directory (again roxygen2)
  9. run devtools::document(roclets = c('rd', 'collate', 'namespace'))
  10. check fsttester package with devtools::check(cleanup = FALSE)

The check runs without errors on my (Win10 and Ubuntu) systems. Then I can run:

fsttester::openmp_info()
#> OpenMP enabled   :  TRUE 
#> Number of threads:  8
Session info
devtools::session_info()
#> Session info -------------------------------------------------------------
#>  setting  value                       
#>  version  R version 3.4.2 (2017-09-28)
#>  system   x86_64, mingw32             
#>  ui       RTerm                       
#>  language (EN)                        
#>  collate  English_United States.1252  
#>  tz       Europe/Berlin               
#>  date     2017-12-02
#> Packages -----------------------------------------------------------------
#>  package   * version date       source        
#>  backports   1.1.1   2017-09-25 CRAN (R 3.4.1)
#>  base      * 3.4.2   2017-09-28 local         
#>  compiler    3.4.2   2017-09-28 local         
#>  datasets  * 3.4.2   2017-09-28 local         
#>  devtools    1.13.4  2017-11-09 CRAN (R 3.4.2)
#>  digest      0.6.12  2017-01-27 CRAN (R 3.4.2)
#>  evaluate    0.10.1  2017-06-24 CRAN (R 3.4.1)
#>  fst         0.7.3   2017-12-02 local         
#>  fsttester   0.1.0   2017-12-02 local         
#>  graphics  * 3.4.2   2017-09-28 local         
#>  grDevices * 3.4.2   2017-09-28 local         
#>  htmltools   0.3.6   2017-04-28 CRAN (R 3.4.1)
#>  knitr       1.17    2017-08-10 CRAN (R 3.4.1)
#>  magrittr    1.5     2014-11-22 CRAN (R 3.4.2)
#>  memoise     1.1.0   2017-04-21 CRAN (R 3.4.2)
#>  methods   * 3.4.2   2017-09-28 local         
#>  parallel    3.4.2   2017-09-28 local         
#>  Rcpp        0.12.14 2017-11-23 CRAN (R 3.4.2)
#>  rmarkdown   1.8     2017-11-17 CRAN (R 3.4.2)
#>  rprojroot   1.2     2017-01-16 CRAN (R 3.4.2)
#>  stats     * 3.4.2   2017-09-28 local         
#>  stringi     1.1.6   2017-11-17 CRAN (R 3.4.2)
#>  stringr     1.2.0   2017-02-18 CRAN (R 3.4.2)
#>  tools       3.4.2   2017-09-28 local         
#>  utils     * 3.4.2   2017-09-28 local         
#>  withr       2.1.0   2017-11-01 CRAN (R 3.4.2)
#>  yaml        2.1.14  2016-11-12 CRAN (R 3.4.1)

I'm curious to see if thefsttester package will work on your system!

greetings and thanks for testing

@eipi10
Copy link
Author

eipi10 commented Dec 2, 2017

Hi Marcus,

My copy of fst is built with OpenMP support. When I tried to document or build fsttester I got the same crashing behavior. I also changed to #' @import data.table with my test package and I experienced the same crashing behavior as when I imported fst. Finally, I tried all this on another Macbook Pro that does not have OpenMP installed and I did not get any crashes.

So, as you surmised, it looks like it's related to OSX systems with OpenMP.

Any ideas on how to resolve this? It looks like I can build packages that don't import fst and just use the fst::read.fst() idiom instead. But it would be nice to be able to resolve the underlying problem.

Thanks for your help.

Joel

@MarcusKlik
Copy link
Collaborator

MarcusKlik commented Dec 3, 2017

Hi @eipi10, thanks for testing! Indeed the fact that data.table also crashes seems to point to OpenMP issues, did you use the instructions on the data.table wiki to install an OpenMP enabled clang compiler?

Your issues seems related to this issue in the data.table repository. The strange thing is that your session crashes when you attach fst (or data.table) but not when you just load it. Attaching runs the onAttach method and that method runs some code that depends on OpenMP.

You could try to comment-out the code in that method (in a fresh fst or data.table clone) and build it. If you can then build your package that imports fst (or data.table) without problems, we would at least have isolated the problem.

(but not solved it yet :-))

@eipi10
Copy link
Author

eipi10 commented Dec 3, 2017

I installed OpenMP (can't remember if that was part of a clang installation) a few months ago in order to take advantage of multiple cores with fst. Getting OpenMP (or clang with OpenMP or whatever it was) to install was a pain--lots of searching around for the correct incantations on the Mac command line, but I really can't remember where I found the right commands or what they were.

I've been using fst in my scripts for a while now without any problems (and when I run library(fst) it outputs "OpenMP detected, using 4 cores"). It's only when importing fst into another package that any issues have turned up.

@MarcusKlik
Copy link
Collaborator

Yes, so nothing strange going on until the #' @import fst. Perhaps it's related to the roxygen2 package, did you try to build without the roxygen2 option in RStudio just to check?

image

greetings

@PeteHaitch
Copy link

PeteHaitch commented Dec 11, 2017

FWIW I'm experiencing what I think is the same issue. Building my fstArray package () using devtools::build() or running devtools::document() leads to segfaults and aborted Rstudio sessions (the fst_develop branch of fstArray uses the develop branch of fst, whereas master of fstArray uses the current CRAN release of fst).

I am on macOS 10.12.6, running a recent-ish build of R-devel with OpenMP supported via clang 4.0 (installed following instructions at https://cran.r-project.org/bin/macosx/tools/), and I installed fst using devtools::install_github("fstpackage/fst@develop") (confirmed that I am seeing -fopenmp compiler flags during installation).

Unfortunately, in my case, resorting to fst::foo() in package code instead of importFrom(fst,foo) in the NAMESPACE does not resolve the issue.

I'm very happy to help out with further testing but won't have time until later this week.

Thanks for your great work on fst, Marcus!
Pete

Session info
#> sessionInfo()
R Under development (unstable) (2017-10-30 r73642)
Platform: x86_64-apple-darwin15.6.0 (64-bit)
Running under: macOS Sierra 10.12.6

Matrix products: default
BLAS: /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib
LAPACK: /Library/Frameworks/R.framework/Versions/3.5/Resources/lib/libRlapack.dylib

locale:
[1] en_AU.UTF-8/en_AU.UTF-8/en_AU.UTF-8/C/en_AU.UTF-8/en_AU.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] colorout_1.1-3    repete_0.0.0.9010 devtools_1.13.4  

loaded via a namespace (and not attached):
 [1] Rcpp_0.12.14            whisker_0.3-2           knitr_1.17              XVector_0.19.1         
 [5] magrittr_1.5            GenomicRanges_1.31.1    BiocGenerics_0.25.0     zlibbioc_1.25.0        
 [9] IRanges_2.13.4          munsell_0.4.3           colorspace_1.3-2        stringr_1.2.0          
[13] GenomeInfoDb_1.15.1     plyr_1.8.4              tools_3.5.0             parallel_3.5.0         
[17] clipr_0.4.0             withr_2.1.0             htmltools_0.3.6         fortunes_1.5-4         
[21] rprojroot_1.2           yaml_2.1.15             digest_0.6.12           GenomeInfoDbData_0.99.2
[25] callr_1.0.0             pryr_0.1.3              S4Vectors_0.17.12       bitops_1.0-6           
[29] codetools_0.2-15        RCurl_1.95-4.8          evaluate_0.10.1         memoise_1.1.0          
[33] rmarkdown_1.8           reprex_0.1.1            stringi_1.1.6           compiler_3.5.0         
[37] backports_1.1.1         scales_0.5.0            stats4_3.5.0   

#> devtools::session_info()
Session info ------------------------------------------------------------------------
 setting  value                                             
 version  R Under development (unstable) (2017-10-30 r73642)
 system   x86_64, darwin15.6.0                              
 ui       RStudio (1.1.379)                                 
 language (EN)                                              
 collate  en_AU.UTF-8                                       
 tz       America/New_York                                  
 date     2017-12-11                                        

Packages ----------------------------------------------------------------------------
 package          * version    date       source                                     
 base             * 3.5.0      2017-10-31 local                                      
 BiocGenerics     * 0.25.0     2017-10-31 Bioconductor                               
 bitops             1.0-6      2013-08-17 CRAN (R 3.5.0)                             
 codetools          0.2-15     2016-10-05 CRAN (R 3.5.0)                             
 colorout         * 1.1-3      2017-10-31 Github (jalvesaq/colorout@e2a175c)         
 colorspace         1.3-2      2016-12-14 CRAN (R 3.5.0)                             
 commonmark         1.4        2017-09-01 CRAN (R 3.5.0)                             
 compiler           3.5.0      2017-10-31 local                                      
 datasets         * 3.5.0      2017-10-31 local                                      
 DelayedArray     * 0.5.7      2017-12-06 Bioconductor                               
 devtools         * 1.13.4     2017-11-09 CRAN (R 3.5.0)                             
 digest             0.6.12     2017-01-27 CRAN (R 3.5.0)                             
 fortunes           1.5-4      2016-12-29 CRAN (R 3.5.0)                             
 fst                0.7.3      2017-12-11 Github (fstpackage/fst@e060e62)            
 fstArray         * 0.0.0.9005 <NA>       Bioconductor                               
 GenomeInfoDb       1.15.1     2017-11-12 Bioconductor                               
 GenomeInfoDbData   0.99.2     2017-11-13 Bioconductor                               
 GenomicRanges      1.31.1     2017-11-17 Github (Bioconductor/GenomicRanges@d65bed4)
 graphics         * 3.5.0      2017-10-31 local                                      
 grDevices        * 3.5.0      2017-10-31 local                                      
 IRanges          * 2.13.4     2017-11-22 Bioconductor                               
 magrittr           1.5        2014-11-22 CRAN (R 3.5.0)                             
 matrixStats      * 0.52.2     2017-04-14 CRAN (R 3.5.0)                             
 memoise            1.1.0      2017-04-21 CRAN (R 3.5.0)                             
 methods          * 3.5.0      2017-10-31 local                                      
 munsell            0.4.3      2016-02-13 CRAN (R 3.5.0)                             
 parallel         * 3.5.0      2017-10-31 local                                      
 plyr               1.8.4      2016-06-08 CRAN (R 3.5.0)                             
 pryr               0.1.3      2017-10-30 CRAN (R 3.5.0)                             
 R6                 2.2.2      2017-06-17 CRAN (R 3.5.0)                             
 Rcpp               0.12.14    2017-11-23 CRAN (R 3.5.0)                             
 RCurl              1.95-4.8   2016-03-01 CRAN (R 3.5.0)                             
 repete           * 0.0.0.9010 2017-11-24 Github (PeteHaitch/repete@db2caea)         
 roxygen2           6.0.1      2017-02-06 CRAN (R 3.5.0)                             
 S4Vectors        * 0.17.12    2017-12-02 cran (@0.17.12)                            
 scales             0.5.0      2017-08-24 CRAN (R 3.5.0)                             
 stats            * 3.5.0      2017-10-31 local                                      
 stats4           * 3.5.0      2017-10-31 local                                      
 stringi            1.1.6      2017-11-17 CRAN (R 3.5.0)                             
 stringr            1.2.0      2017-02-18 CRAN (R 3.5.0)                             
 tools              3.5.0      2017-10-31 local                                      
 utils            * 3.5.0      2017-10-31 local                                      
 withr              2.1.0      2017-11-01 CRAN (R 3.5.0)                             
 xml2               1.1.1      2017-01-24 CRAN (R 3.5.0)                             
 XVector            0.19.1     2017-11-17 Bioconductor                               
 yaml               2.1.15     2017-12-01 CRAN (R 3.5.0)                             
 zlibbioc           1.25.0     2017-10-31 Bioconductor 

@MarcusKlik
Copy link
Collaborator

MarcusKlik commented Dec 11, 2017

Hi @PeteHaitch, thanks for reporting and your fstArray is a very nice package, interesting!

I guess the ideas we shared in issue #19 are still very relevant to optimize the speed in your package? If the speed is now comparable to HDF5Array then implementing the blocked format would surely make it a lot faster :-).

It would take a series of memcpy operations to first cut the (basically 1-dimensional) R matrix to pieces, but we could do that in parallel. Then before serializing, we can apply compression. The operations wouldn't be too far off what is already done for 'normal' columns, only the initial memcpy operations are an extra step. If we can make that fast enough, it would definitely be a very interesting addition to the fst package!

Anyway, these OpenMP problems on macOS are quite troublesome. So your master branch has no problems because the current CRAN version has no OpenMP dependencies. It guess we could still test a couple of things:

  • When fst v0.8.0 is released (any time now), the CRAN version will have OpenMP multi-threading (CRAN builds with OpenMP for mac binaries). I'm curious to see if you will experience these problems on your master branch after that release.
  • We should build a debug version of fst and then compile your package. While using devtools::build() we can attach to the fst library and see what happens.
  • To start with, we could also remove the OpenMP dependent code from the .onAttach method in the fst package and then try to build your package. Perhaps problems only arise during the attach phase.

Do you have the same problems if you attach the data.table package with your setup (like @eipi10) ? What happens if you build your package from the command line, without using RStudio?

greetings

@PeteHaitch
Copy link

Hi @MarcusKlik,

Yes, #19 would be brilliant! I decided in the meanwhile to experiment with supporting matrix-like data that are backed via an fst data frame. The good news is that I'm finding it very promising: a common data access pattern I face as a package developer and user is to access contiguous rows from arbitrary columns of a 2-dimensional matrix-like object, and the current implementation really shines here (perhaps, unsurprisingly).

Short term, my plan is to submit fstArray to Bioconductor following the next CRAN release of fst; even with this somewhat limited features I think it will be useful. Longer term, I hope to use a more matrix/-like fst backend and even extend to multidimensional arrays. Obviously, native support for multidimensional arrays in fst would be a huge help to me. My (more ambitious) plan is to leverage https://github.com/fstpackage/fstlib/ and develop the array infrastructure myself. This would be a great learning opportunity, but rather ambitious from my point of view.

When fst v0.8.0 is released (any time now), the CRAN version will have OpenMP multi-threading (CRAN builds with OpenMP for mac binaries). I'm curious to see if you will experience these problems on your master branch after that release.
We should build a debug version of fst and then compile your package. While using devtools::build() we can attach to the fst library and see what happens.
To start with, we could also remove the OpenMP dependent code from the .onAttach method in the fst package and then try to build your package. Perhaps problems only arise during the attach phase.

To return to the OpenMP issues, my current plan is to first wait for the imminent v0.8.0 release
. Regardless of whether or not that fixes my issue, I'd be happy to help with (2) or (3) to debug fst on macOS.

Do you have the same problems if you attach the data.table package with your setup (like @eipi10) ? What happens if you build your package from the command line, without using RStudio?

I'll try to find time in this week to test these out.

Thanks,
Pete

@PeteHaitch
Copy link

Quick update:

  1. This seems to be RStudio-specific issue. Running devtools::load_all(), devtools::document() etc. from command line R on the same machine works perfectly well for me. Where the root cause lies (e.g., RStudio itself or RStudio catching a glitch in devtools or fst) is unclear to me, but I'm happy to keep digging.
  2. I realised that installing v0.8.0 from CRAN won't help in my case since I am running R-devel on macOS, which means all CRAN packages are installed from source (CRAN don't provide macOS binaries for R-devel)

Pete

@MarcusKlik
Copy link
Collaborator

Hi @PeteHaitch, thanks for the update and a (small) relief that you can build from the command line, even with devtools. Perhaps the devtools integration with RStudio causes some problems in combination with OpenMP. If you run devtools::load_all() not from the RStudio button but just type it in the console, does that work without problems?

@PeteHaitch
Copy link

PeteHaitch commented Dec 14, 2017

Okay, I think I'm getting close to the source of the problem. I was unable to reproduce the issue when calling any of devtools::document(), devtools::load_all(), devtools::build(), or devtools::check() from the Rstudio console. However, when I used the keyboard shortcuts (e.g., ⇧⌘E on a Mac) I am (intermittently) hit by the 'R Session Aborted' error (show below)

image

Triggering the error tends to require a series of devtools commands called via Rstudio keyboard shortcuts. I haven't figured out the exact sequence.

I'm going to propose submitting this as a bug report to RStudio. First, I will need to confirm that I can trigger it using the toy package you posted in #109 (comment)

@MarcusKlik
Copy link
Collaborator

Hi @PeteHaitch, good find and nice theme by the way 👍. Yes, so it seems like an RStudio bug after all, that's good to know, thanks for sharing!

@PeteHaitch
Copy link

Following up on #109 (comment):

I'm going to propose submitting this as a bug report to RStudio. First, I will need to confirm that I can trigger it using the toy package you posted

I can confirm this can be triggered by the toy package. I've posted to rstudio/rstudio#1920

@MarcusKlik
Copy link
Collaborator

Thanks @PeteHaitch, I'm curious to see if the RStudio people understand what is causing this problem!

@eipi10
Copy link
Author

eipi10 commented Jan 8, 2018

Marcus,

Following up on your question from December 3 (sorry for the delay in getting back to you): When I unchecked the Roxygen option and then ran devtools::document(), RStudio still crashed.

Joel

@kevinushey
Copy link

I was able to capture a stack trace in RStudio, following the steps from @PeteHaitch at rstudio/rstudio#1920.

libsystem_platform.dylib`_os_unfair_lock_unowned_abort:
    0x7fff702575e4 <+0>:  movl   %edi, %eax
    0x7fff702575e6 <+2>:  leaq   0x246a(%rip), %rcx        ; "BUG IN CLIENT OF LIBPLATFORM: Unlock of an os_unfair_lock not owned by current thread"
    0x7fff702575ed <+9>:  movq   %rcx, 0x38de0ccc(%rip)    ; gCRAnnotations + 8
Target 0: (rsession) stopped.

(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
    frame #0: 0x00007fff702575e2 libsystem_platform.dylib`_os_unfair_lock_recursive_abort + 23
libsystem_platform.dylib`_os_unfair_lock_recursive_abort:
->  0x7fff702575e2 <+23>: ud2

libsystem_platform.dylib`_os_unfair_lock_unowned_abort:
    0x7fff702575e4 <+0>:  movl   %edi, %eax
    0x7fff702575e6 <+2>:  leaq   0x246a(%rip), %rcx        ; "BUG IN CLIENT OF LIBPLATFORM: Unlock of an os_unfair_lock not owned by current thread"
    0x7fff702575ed <+9>:  movq   %rcx, 0x38de0ccc(%rip)    ; gCRAnnotations + 8
Target 0: (rsession) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
  * frame #0: 0x00007fff702575e2 libsystem_platform.dylib`_os_unfair_lock_recursive_abort + 23
    frame #1: 0x00007fff70257710 libsystem_platform.dylib`_os_unfair_lock_lock_slow + 226
    frame #2: 0x00007fff7025d69e libsystem_pthread.dylib`pthread_atfork + 73
    frame #3: 0x000000011e96c57b libomp.dylib`__kmp_register_atfork + 62
    frame #4: 0x000000011e944d3c libomp.dylib`__kmp_do_serial_initialize() + 788
    frame #5: 0x000000011e94ab33 libomp.dylib`__kmp_do_middle_initialize() + 27
    frame #6: 0x000000011e94ab01 libomp.dylib`__kmp_middle_initialize + 52
    frame #7: 0x000000011e972b99 libomp.dylib`omp_get_max_threads + 21
    frame #8: 0x000000011e617891 fst.so`GetFstThreads() + 17
    frame #9: 0x000000011e6178cd fst.so`SetFstThreads(int) + 13
    frame #10: 0x00007fff70263f21 libsystem_pthread.dylib`_pthread_atfork_prepare_handlers + 90
    frame #11: 0x00007fff6ddcfa2d libSystem.B.dylib`libSystem_atfork_prepare + 11
    frame #12: 0x00007fff7003281b libsystem_c.dylib`fork + 12
    frame #13: 0x00000001007b6726 rsession`rstudio::core::Error rstudio::core::system::posixCall<int>(rstudio_boost::function<int ()> const&, rstudio::core::ErrorLocation const&, int*) + 70
    frame #14: 0x00000001007b2225 rsession`rstudio::core::system::ChildProcess::run() + 2469
    frame #15: 0x00000001007702b4 rsession`rstudio::core::system::AsyncChildProcess::run(rstudio::core::system::ProcessCallbacks const&) + 36
    frame #16: 0x000000010076a298 rsession`rstudio::core::system::(anonymous namespace)::runChild(rstudio_boost::shared_ptr<rstudio::core::system::AsyncChildProcess>, std::vector<rstudio_boost::shared_ptr<rstudio::core::system::AsyncChildProcess>, std::allocator<rstudio_boost::shared_ptr<rstudio::core::system::AsyncChildProcess> > >*, rstudio::core::system::ProcessCallbacks const&) + 56
    frame #17: 0x000000010076a47a rsession`rstudio::core::system::ProcessSupervisor::runCommand(std::string const&, rstudio::core::system::ProcessOptions const&, rstudio::core::system::ProcessCallbacks const&) + 138
    frame #18: 0x00000001003b826b rsession`rstudio::session::modules::build::(anonymous namespace)::Build::roxygenize(rstudio::core::FilePath const&, rstudio::core::system::ProcessOptions, rstudio::core::system::ProcessCallbacks const&) + 4139
    frame #19: 0x00000001003b16b7 rsession`rstudio::session::modules::build::(anonymous namespace)::Build::executePackageBuild(std::string const&, rstudio::core::FilePath const&, rstudio::core::system::ProcessOptions const&, rstudio::core::system::ProcessCallbacks const&) + 2503
    frame #20: 0x00000001003adca6 rsession`rstudio::session::modules::build::(anonymous namespace)::Build::executeBuild(std::string const&, std::string const&, rstudio::core::system::ProcessCallbacks const&) + 1190
    frame #21: 0x00000001003a98fd rsession`rstudio::session::modules::build::(anonymous namespace)::startBuild(rstudio::core::json::JsonRpcRequest const&, rstudio::core::json::JsonRpcResponse*) + 3789
    frame #22: 0x0000000100070624 rsession`rstudio_boost::detail::function::function_invoker2<rstudio::core::Error (*)(rstudio::core::json::JsonRpcRequest const&, rstudio::core::json::JsonRpcResponse*), rstudio::core::Error, rstudio::core::json::JsonRpcRequest const&, rstudio::core::json::JsonRpcResponse*>::invoke(rstudio_boost::detail::function::function_buffer&, rstudio::core::json::JsonRpcRequest const&, rstudio::core::json::JsonRpcResponse*) + 20
    frame #23: 0x000000010064f37e rsession`rstudio::core::json::(anonymous namespace)::runSynchronousFunction(rstudio_boost::function<rstudio::core::Error (rstudio::core::json::JsonRpcRequest const&, rstudio::core::json::JsonRpcResponse*)> const&, rstudio::core::json::JsonRpcRequest const&, rstudio_boost::function<void (rstudio::core::Error const&, rstudio::core::json::JsonRpcResponse*)> const&) + 94
    frame #24: 0x00000001000eefdf rsession`rstudio::session::rpc::handleRpcRequest(rstudio::core::json::JsonRpcRequest const&, rstudio_boost::shared_ptr<rstudio::session::HttpConnection>, rstudio::session::http_methods::ConnectionType) + 991
    frame #25: 0x00000001000f426c rsession`rstudio::session::http_methods::handleConnection(rstudio_boost::shared_ptr<rstudio::session::HttpConnection>, rstudio::session::http_methods::ConnectionType) + 1180
    frame #26: 0x00000001000f3207 rsession`rstudio::session::http_methods::waitForMethod(std::string const&, rstudio_boost::function<void ()> const&, rstudio_boost::function<bool ()> const&, rstudio::core::json::JsonRpcRequest*) + 2375
    frame #27: 0x0000000100033f49 rsession`rstudio::session::console_input::rConsoleRead(std::string const&, bool, rstudio::r::session::RConsoleInput*) + 649
    frame #28: 0x000000010082857c rsession`rstudio::r::session::RReadConsole(char const*, unsigned char*, int, int) + 892
    frame #29: 0x0000000101460d6b libR.dylib`Rf_ReplIteration(rho=0x000062500005af40, savestack=0, browselevel=0, state=0x00007ffeefbfc680) at main.c:206 [opt]
    frame #30: 0x0000000101465161 libR.dylib`R_ReplConsole(rho=0x000062500005af40, savestack=0, browselevel=0) at main.c:308 [opt]
    frame #31: 0x0000000101464f38 libR.dylib`run_Rmainloop at main.c:1082 [opt]
    frame #32: 0x000000010084fc89 rsession`rstudio::r::session::runEmbeddedR(rstudio::core::FilePath const&, rstudio::core::FilePath const&, bool, bool, SA_TYPE, rstudio::r::session::Callbacks const&, rstudio::r::session::InternalCallbacks*) + 425
    frame #33: 0x000000010082ec8c rsession`rstudio::r::session::run(rstudio::r::session::ROptions const&, rstudio::r::session::RCallbacks const&) + 3164
    frame #34: 0x0000000100108c73 rsession`main + 28963
    frame #35: 0x00000001000038c4 rsession`start + 52

Potentially related issues:

This is the commit that worked around the associated issue in data.table; perhaps fst needs something similar?

Rdatatable/data.table@9d1f3e2

@kevinushey
Copy link

I think the issue comes down to the use of OpenMP within forked processes. There's a small discussion of potential issues here:

https://bisqwit.iki.fi/story/howto/openmp/#OpenmpAndFork

Basically, if you have a process that might call ::fork() (which RStudio does, to launch child R processes during e.g. devtools::document()), then one must ensure that OpenMP features are not used within the forked process.

If I understand correctly, either the parent or the child processes may use OpenMP features, but not both.

@MarcusKlik
Copy link
Collaborator

Hi @kevinushey, thanks for investigating that and for the references to relevant data.table issues,

I studied the code in data.table that switches back to single threaded mode after a fork and believed that I had implemented the same protection mechanism in fst, but perhaps I missed something or data.table has further updates that solves the problem.

Good to learn that devtools::document() is executed from a fork, thanks for all the information!

@MarcusKlik MarcusKlik added this to the fst v0.8.6 milestone Jan 9, 2018
@kevinushey
Copy link

kevinushey commented Jan 9, 2018

Just to be specific, RStudio launches R processes in the build pane through a fork -- for example, when you click the Document button, we launch a fresh R process (by forking the rsession) and then execute devtools::document(). In other words, it's not devtools doing the forking; RStudio as a front-end is doing that to get a clean-slate R process launched.

@MarcusKlik
Copy link
Collaborator

Yes, exactly, That fork should be detected with this code which sets the number of threads to one. That's basically the same setup as the fork protection in data.table (or so I thought :-))

Thanks, I will study the problem and report if I have more information!

@MarcusKlik
Copy link
Collaborator

Hi @kevinushey, the forking issues with the fst package and RStudio have been solved. To summarize our findings (with thanks to @PeteHaitch and @thierrygosselin in issue #100 ):

  • It seems that any call to the OpenMP library causes problems. The fix in data.table removed the call to omp_set_num_threads and that solved the problem for the data.table package. fst was also calling the OpenMP method omp_get_max_threads() from the fork (inside the when_fork method):
pthread_atfork(&when_fork, NULL, NULL)

that crashes RStudio when it launches a forked process (to call devtools::document() for example).

  • Removing all calls to OpenMP methods fixes the issue.

Thanks for the information and helping to track down and fix the problem!

@MarcusKlik
Copy link
Collaborator

Hi @eipi10, this fix should solve your issues as well, if you are still experiencing problem, please let me know!

@MarcusKlik MarcusKlik modified the milestones: fst v0.8.6, fst v0.8.4 Jan 23, 2018
@kevinushey
Copy link

Awesome news! Thank you so much for investigating and taking the time to get a fix @MarcusKlik.

@eipi10
Copy link
Author

eipi10 commented Jan 23, 2018

Thanks very much for investigating and fixing this Marcus!

@MarcusKlik
Copy link
Collaborator

MarcusKlik commented Jan 23, 2018

No problem!

(and being able to build packages with RStudio/macOS with fst as dependency is definitely top priority :-))

thierrygosselin added a commit to thierrygosselin/radiator that referenced this issue Jan 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants