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

Possible memory leak reactiveFileReader #2321

Closed
RichardHooijmaijers opened this issue Feb 6, 2019 · 11 comments
Closed

Possible memory leak reactiveFileReader #2321

RichardHooijmaijers opened this issue Feb 6, 2019 · 11 comments

Comments

@RichardHooijmaijers
Copy link

RichardHooijmaijers commented Feb 6, 2019

I know there are a couple of issues that are linked (e.g #1551 #2021) but thought this might be considered as a new issue.

If I run a slightly adapted version of the reactiveFileReader example, my memory seems to increase quite a bit when the app runs for a longer time (e.g. >12h). This happens with the following code:

library(shiny)
ui <- fluidPage(
  verbatimTextOutput("monitor_txt")
)
server <- function(input, output, session) {
  monitor_out <- reactiveFileReader(500,session,"memtext.txt", readLines)
  output$monitor_txt <- renderText({paste(monitor_out(), collapse = '\n')})
}
shinyApp(ui, server,options=list(launch.browser=TRUE))

The memory increase by a couple of hundred mb in 12h. Something similar is seen when I read in the file 'manually' in combination with invalidateLater. The file being read is not very large (10-100kb). This was tested in R 3.4.2 with shiny 1.2.0 on ubuntu 16.04

Any help on this is would b very helpful!

@Jingadilian
Copy link

Jingadilian commented Mar 20, 2019

@RichardHooijmaijers thank you so much for posting this issue!

I have been trying to figure out for ages now why my app increased in memory usage even when idle. For example, after sitting idle for one hour it usually increased by 500mb.

Preliminary results suggests that removing my 5 x reactiveFileReader has fixed the issue.

@shrektan
Copy link
Contributor

shrektan commented Mar 20, 2019

I have the same experience. One of my app uses reactiveFileReader(). The memory starts from 100mb. After 12 or 24 hours, it bumps to 1gb or more. Adding a gc() doesn’t help. So I doubt it’s a memory leak.
update: Also notice some other long time running R processes (hosted in Docker) occupy multiple times memory than expected. So it may not be related to Shiny. Ignore me please...

@alandipert
Copy link
Contributor

alandipert commented Mar 30, 2019

I think I saw something like this too. I made the following app to demonstrate the increased memory usage over time, and the fact that gc() doesn't appear to help:

library(shiny)

ui <- fluidPage(
  verbatimTextOutput("stats"),
  actionButton("gcButton", "GC")
)

server <- function(input, output, session) {
  continue <- TRUE
  hist <- reactiveValues(
    start_time = Sys.time(),
    start_mem = as.numeric(pryr::mem_used()),
    current_time = Sys.time(),
    current_mem = as.numeric(pryr::mem_used())
  )
  
  poll <- function() {
    if (continue) {
      cat("foo\n", file = "somefile")
      later::later(poll, 1)
    }
  }
  
  poll()
  
  onStop(function() { 
    continue <<- FALSE
    unlink("somefile")
  })
  
  reader <- reactiveFileReader(500, session, "somefile", readLines)
  
  observeEvent(reader(), {
    hist$current_mem <- as.numeric(pryr::mem_used())
    hist$current_time <- Sys.time()
  })
  
  output$stats <- renderText({
    sprintf("Start time: %s\nStart mem: %s\nCurrent time: %s\nCurrent mem: %s",
      toString(hist$start_time),
      utils:::format.object_size(hist$start_mem, "auto"),
      toString(hist$current_time),
      utils:::format.object_size(hist$current_mem, "auto")
    )
  })
  
  observeEvent(input$gcButton, {
    cat("GCing...\n")
    gc()
  })
}

shinyApp(ui, server,options=list(launch.browser=TRUE))

@alandipert
Copy link
Contributor

Worked with @jcheng5 and arrived at a smaller repro:

setInterval <- function(func, interval) {
  poll <- function() {
    func()
    later::later(poll, interval)
  }
  poll()
}

setInterval(function() {
  message(pryr::mem_used())
}, 2)

library(shiny)

rv <- reactiveValues(x = 0L)
observe({ rv$x })

setInterval(function() {
  rv$x <- isolate(rv$x) + 1L
  shiny:::flushReact()
}, 0.05)

@jcheng5
Copy link
Member

jcheng5 commented Apr 5, 2019

🤔

# Fill global string pool
for (i in 1:512) {
  paste(i)
}

dependents <- new.env(parent = emptyenv())
while (TRUE) {
  print(pryr::mem_used())
  for (i in 1:1000) {
    x <- as.character(runif(1))
    # If you comment out the next line, no leak...
    exists(x, envir = dependents, inherits = FALSE)
  }
}
─ Session info ────────────────────────────────────────────────────────────────────────────────
 setting  value                                      
 version  R version 3.5.1 Patched (2018-10-15 r75450)
 os       macOS Sierra 10.12.3                       
 system   x86_64, darwin15.6.0                       
 ui       RStudio                                    
 language (EN)                                       
 collate  en_US.UTF-8                                
 ctype    en_US.UTF-8                                
 tz       America/Los_Angeles                        
 date     2019-04-04                                 

Also reproduced in R 3.3.3/Linux

@alandipert
Copy link
Contributor

alandipert commented Apr 5, 2019

I haven't looked at where we use exists and so I can't contribute any solution ideas yet, but I think the problem is: exists makes a symbol out of its argument and that symbol is interned in the global symbol table and remains there indefinitely.

  1. base::exists() calls the the .Internal function exists(), which maps to the C function do_get
  2. do_get calls installTrChar
  3. installTrChar calls install
  4. install adds the new symbol to the global symbol table R_SymbolTable
  5. The symbol is never "un-interned" and so R_SymbolTable grows indefinitely.
pryr::mem_change(lapply(as.character(runif(10000)), exists))

@jcheng5
Copy link
Member

jcheng5 commented Apr 5, 2019

I suspect we'll need to replace everywhere we're using environments as maps. Unfortunately we don't have a great candidate for replacement right now. I prototyped a naive impl that uses Rcpp to wrap a std::map, which worked, but the performance is much worse than with environments.

@wch
Copy link
Collaborator

wch commented Sep 13, 2019

It turns out there were two sources of leakage that are relevant here, both of which are fixed now: #2429 and #2522.

@wch wch closed this as completed Sep 13, 2019
@Lukasyo
Copy link

Lukasyo commented Oct 15, 2019

Hi, I encountered the same problem with reactiveFileReader(). Thanks to @wch I understand that it is fixed by using fastmap as backing store for Map class #2429 . However it is not clear to me, how it is used in the case of reactiveFileReader().

Help would be very much appreciated as I am currently surviving this by cron rebooting server 5 times a day.

@shrektan
Copy link
Contributor

Updating shiny package to the latest CRAN version (1.4.0) should be enough, isn't it?

@Lukasyo
Copy link

Lukasyo commented Nov 14, 2019

@shrektan , yes it is enough, thanks!

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

No branches or pull requests

7 participants