Fix reactivePoll leak

[wch]

This fixes #1548. It allows reactivePoll objects to clean up when there are no longer any references to them.

Note that there must no longer be any references to the reactivePoll object for it to be cleaned up. If there's still a reference to the object, even if that reference isn't consumed by any other reactives/observers, that's not sufficient for it to be cleaned up; in other words, it will keep firing.

Example app:

library(shiny)
shinyApp(
  fluidPage(
    p("Value of ", code("count")),
    verbatimTextOutput("count"),
    p("Value of ", code("i()")),
    verbatimTextOutput("i"),
    actionButton("replace", "Replace count()")
  ),
  function(input, output, session) {
    i <- reactiveVal(0)
    count <- reactivePoll(1000, session,
      checkFunc = function() {
        isolate({
          i(i() + 1)
          i()
        })
      },
      valueFunc = function() {
        isolate(i())
      }
    )

    output$count <- renderText({
      count()
    })
    output$i <- renderText({
      i()
    })

    observeEvent(input$replace, {
      count <<- reactive("count() has been replaced")
      gc()
    })
  }
)

Here's how it behaves before this PR:

And after this PR:

Note that before the change, the replacement reactive, which has the value "count() has been replaced", shows up in output$count, but after the change, it doesn't. This is because, before the change, the invalidation keeps happening on the timer and causes the renderText to re-execute; but after the change, the invalidation stops happening after count() is replaced, so it does not cause the renderText to re-execute.

Here's a test that works outside of a Shiny app:

library(shiny)
i <- 0
count <- reactivePoll(100, NULL,
  checkFunc = function() {
    i <<- i + 1
    i
  },
  valueFunc = function() i
)

observe({
  message(count())
})
while(i < 3) {
  Sys.sleep(0.1)
  shiny:::timerCallbacks$executeElapsed()
  shiny:::flushReact()
}

rm(count)
gc()

# If the reactivePoll was cleaned up, then running the following should no
# longer increment i.
Sys.sleep(0.2)
shiny:::timerCallbacks$executeElapsed()
shiny:::flushReact()
Sys.sleep(0.2)
shiny:::timerCallbacks$executeElapsed()
shiny:::flushReact()

# This is the crucial part
stopifnot(i == 3)

Update: With the final version of this PR, in test app, i() will go one number higher than count -- what's important is that i() stops at some point. Also, in the test code for outside of a Shiny app, it should check for stopifnot(i == 3) (this is in the unit tests in the PR).

[jcheng5]

After in-person discussion: Let's change the finalizer to merely set a boolean variable, that the observer will look at and destroy itself when it is set to TRUE.

[jcheng5]

The missing return()