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

Strange behavior when assigning values in Environment, due to GC #780

Closed
wch opened this issue Dec 1, 2017 · 7 comments · Fixed by #784
Closed

Strange behavior when assigning values in Environment, due to GC #780

wch opened this issue Dec 1, 2017 · 7 comments · Fixed by #784

Comments

@wch
Copy link

wch commented Dec 1, 2017

I've been running into a very hard-to-debug problem which I believe is due to a GC during assignment to an Environment. In some cases, the result of assignment into an Environment has the wrong type, and in other cases, the Environment somehow gets lost.

Here is an example to run in a clean R session:

library(Rcpp)
cppFunction(
  code = '
  Environment f() {
    Environment env = Function("new.env")();
    env["A"] = "a";
    env["B"] = "b";
    env["C"] = "c";
    env["FOO"] = "foo";
    return env;
  }
  '
)

gctorture(TRUE)
e <- f()
gctorture(FALSE)
e$A
e$B
e$C
e$FOO

When I run this on my Mac, here's what it prints out for the last commands:

> gctorture(TRUE)
> e <- f()
> gctorture(FALSE)
> e$A
[1] "a"
> e$B
<CHARSXP: "B">
> e$C
[1] "c"
> e$FOO
<CHARSXP: "FOO">

Inspecting the objects gives this:

> .Internal(inspect(e$A))
@7fd168ce9c68 16 STRSXP g1c1 [MARK,NAM(2)] (len=1, tl=0)
  @7fd168868598 09 CHARSXP g1c1 [MARK,gp=0x61] [ASCII] [cached] "a"
> .Internal(inspect(e$B))
@7fd168ce9c98 09 CHARSXP g1c1 [MARK,NAM(2),gp=0x61] [ASCII] [cached] "B"

If I run the code a second time, then the results turn out normal. This is the output from a second run:

> gctorture(TRUE)
> e <- f()
> gctorture(FALSE)
> e$A
[1] "a"
> e$B
[1] "b"
> e$C
[1] "c"
> e$FOO
[1] "foo"

I get the exact same result when I run the same code in the r-base Docker image. To reproduce:

docker run --rm -ti r-base
install.packages('Rcpp')
# Now run the code from above

When I run the same code in a custom build of R-devel with PROTECTCHECK defined, I get something else weird:

....
> gctorture(TRUE)
> e <- f()
> gctorture(FALSE)
> e$A
Error: object 'e' not found
> e$B
Error: object 'e' not found
> e$C
Error: object 'e' not found
> e$FOO
Error: object 'e' not found

e simply never gets assigned.


If the strings get wrapped with CharacterVector(), then the problem doesn't happen. Unlike the code above, this code works fine:

library(Rcpp)
cppFunction(
  code = '
  Environment f() {
    Environment env = Function("new.env")();
    env["A"] = CharacterVector("a");
    env["B"] = CharacterVector("b");
    env["C"] = CharacterVector("c");
    env["FOO"] = CharacterVector("foo");
    return env;
  }
  '
)

gctorture(TRUE)
e <- f()
gctorture(FALSE)
e$A
e$B
e$C
e$FOO

In my particular case, the problem happens with httpuv. When I run the PROTECTCHECK build of R, I get this error in some httpuv code when it assigns a string to an environment:

Error: GC encountered a node (0x5611e376c9d0) with type FREESXP (was STRSXP) at memory.c:1696

The code in question looks like this:

   env["rook.version"] = "1.1-0"

And many other places where a string is assigned results in the same error message. Wrapping all of them in CharacterVector() seems to make the error go away.


If it would help, I can provide the Dockerfile used to create the build of R with PROTECTCHECK defined.

> sessioninfo::session_info()
─ Session info ───────────────────────────────────────────────────────────────
 setting  value                       
 version  R version 3.4.2 (2017-09-28)
 os       macOS High Sierra 10.13.1   
 system   x86_64, darwin15.6.0        
 ui       X11                         
 language (EN)                        
 collate  en_US.UTF-8                 
 tz       <NA>                        
 date     2017-12-01                  

─ Packages ───────────────────────────────────────────────────────────────────
 package     * version date       source        
 clisymbols    1.2.0   2017-05-21 CRAN (R 3.4.0)
 Rcpp        * 0.12.14 2017-11-23 CRAN (R 3.4.3)
 sessioninfo   1.0.0   2017-06-21 CRAN (R 3.4.1)
 withr         2.1.0   2017-11-01 CRAN (R 3.4.2)
@eddelbuettel
Copy link
Member

Phew, that was long. Not sure what the operational approach may be. Tell people to wrap their char * in CharacterVector? Try to find another caster?

What is the status of PROTECTCHECK?

@wch
Copy link
Author

wch commented Dec 1, 2017

I learned that configuring R with --enable-strict-barrier turns on more checks than simply defining PROTECTCHECK. When I use R compiled this way, assigning an integer value into the environment causes the error:

library(Rcpp)
cppFunction(
  code = '
  Environment f() {
    Environment env = Function("new.env")();
    env["A"] = 123;
    return env;
  }
  '
)

gctorture(TRUE)
e <- f()
gctorture(FALSE)

The result:

> e <- f()
Error in f() : 
  unprotected object (0x564466e64e38) encountered (was INTSXP)

Wrapping it with IntegerVector() makes the error go away. Also, if I assign to List instead of Environment, there's no error.

@kevinushey
Copy link
Contributor

Thanks for the investigation! I suspect the error lies here:

set(wrap(rhs));

The call to set(wrap(rhs)) is likely faulty: wrap(rhs) creates an unprotected SEXP, and the implementation of set() for environments + bindings calls into R APIs that can trigger the garbage collector.

I'll see if I can get this fixed up!

@eddelbuettel
Copy link
Member

Interesting. This got closed via @kevinushey wring 'should resolve ...' in the initial comment box. I had thought one needed '(closes ...)' in the title.

@coatless
Copy link
Contributor

coatless commented Dec 6, 2017

Any of the following work:

  • fix{es, ed}
  • resolve{s,d}
  • close{s,d}

c.f. https://help.github.com/articles/closing-issues-using-keywords/

@eddelbuettel
Copy link
Member

In any comment box or just the first? That it works outside of Title: was news to me.

@coatless
Copy link
Contributor

coatless commented Dec 6, 2017

For example, to close an issue numbered 123, you could use the phrases "Closes #123" in your pull request description or commit message. Once the branch is merged into the default branch, the issue will close.

Only in the initial box or as part of the chunk per:
https://help.github.com/articles/closing-issues-using-keywords/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants