rokucommunity / promises

Promise library for roku
MIT License
7 stars 4 forks source link

`promises.isPromise` being called more and more over time #4

Closed arturocuya closed 1 year ago

arturocuya commented 1 year ago

I have set up a sample project for this issue, with the following:

So something like this

sub init()
    m.precondition1 = promises.create()
    m.precondition2 = promises.create()

    m.timer1 = CreateObject("roSGNode", "Timer")
    m.timer1.duration = 3
    m.timer1.ObserveFieldScoped("fire", "handleTimer1")
    m.timer1.control = "start"

    m.timer2 = CreateObject("roSGNode", "Timer")
    m.timer2.duration = 5
    m.timer2.ObserveFieldScoped("fire", "handleTimer2")
    m.timer2.control = "start"
end sub

sub handleTimer1()
    promises.resolve(1, m.precondition1)
    ? "Precondition 1 met"
end sub

sub handleTimer2()
    promises.resolve(2, m.precondition2)
    ? "Precondition 2 met"
end sub

I want to be able to press a button at any moment so that the "hello" task runs only after the 2 preconditions have been met.

I've tried the following approaches:

  1. From the readme examples, use promise.all() and use promises.chain(). This didn't work at all, so you won't see it included in the sample project.
  2. Save the promise returned by promise.all() during init() and reference it when the button is pressed
' (during init)
m.preconditions = promises.all([m.precondition1, m.precondition2])
promises.onThen(m.preconditions, sub(result)
        ? "all preconditions met"
        task = CreateObject("roSGNode", "MyTask")
        taskPromise = task.callFunc("start")
        promises.onThen(taskPromise, sub(result)
            ? "Task completed:", result
        end sub)
    end sub)
  1. Use nested onThens for each precondition and then for the task.
promises.onThen(m.precondition1, sub(result)
        ? "onThen lvl1: Precondition 1 met"
        promises.onThen(m.precondition2, sub(result)
            ? "onThen lvl2: Precondition 2 met"
            task = CreateObject("roSGNode", "MyTask")
            taskPromise = task.callFunc("start")
            promises.onThen(taskPromise, sub(result)
                ? "onThen lvl3: Task completed:", result
            end sub)
        end sub)
    end sub)

The last 2 approaches work, the problem is that if you keep pressing the button, eventually the program crashes due to an execution time out. I've been able to identify that the function promises.isPromise() is somehow related to the problem. I put a print statement in that function to check how many times it's called and every time I press the button, the amount grows similarly to a fibonacci function.

I also discovered that just calling the Task doesn't cause this problem, so maybe it's related to how I'm creating the promises and resolving them in the render thread instead of a task thread?

task = CreateObject("roSGNode", "MyTask")
taskPromise = task.callFunc("start")
promises.onThen(taskPromise, sub(result)
    ? "task only: Task completed:", result
end sub)

Similarly, calling just the preconditions also causes a growth in the number of calls to promise.isPromise()

promises.onThen(m.preconditions, sub(result)
        ? "preconditions only: all preconditions met"
    end sub)

The way I'm testing the number of calls to isPromise() is by opening the app and waiting for the timers to end. At this point it's called around 25 times, which is constant across app resets. Then I start pressing the button and each time I see the number of calls grow (you have to clear the telnet console each time to see the number of prints for each press)

  1. First approach: Doesn't work
  2. Use m.preconditions made from promise.all() -> 12, 13, 15, 19, 27, 43, 75, 139, 267, 523 = a(n) = a(n-1) + a(n-2) + 1
  3. Using nested onThen() -> 19, 23, 31, 47, 79, 143, 271, 527 = a(n) = a(n-1) + a(n-2) + 3
  4. Calling the task directly -> no growth! always 7 prints
  5. Calling the preconditions directly -> 5, 6, 8, 12, 20, 36, 68, 132, 260, 516 = a(n) = a(n-1) + a(n-2) + 2

The attached .zip contains the sample project with all of the approaches listed, ready to be uncommented and tested. You'll need to run npm install first because it uses bsc.

promises-mem-leak.zip

If you spam the button eventually you'll see this execution timeout crash.

BrightScript Micro Debugger.
Enter any BrightScript statement, debug commands, or HELP.
Suspending threads...
Thread selected:  1*   pkg:/source/promises.brs(264)           subpromises_internal_notifyListeners(event as object)
Current Function:
264:* sub promises_internal_notifyListeners(event as object)
265:      originalPromise = event.getRoSgNode()
266:      if promises_isComplete(originalPromise) then
267:          promiseStorage = promises_internal_getPromiseStorage(originalPromise)
268:          promiseState = originalPromise.promiseState
Source Digest(s): 
pkg: dev 1.0.0 446a71f6 promises-mem-leak
Execution timeout (runtime error &h23) in pkg:/source/promises.brs(264)
Backtrace:
#0  Function promises_internal_notifylisteners(event As Object) As Void
   file/line: pkg:/source/promises.brs(265)
Local Variables:
event            roSGNodeEvent refcnt=2
global           Interface:ifGlobal
m                roAssociativeArray refcnt=4 count:25
originalpromise  <uninitialized>
promisestorage   <uninitialized>
promisestate     <uninitialized>
promiseresult    <uninitialized>
listener         <uninitialized>
Threads:
ID    Location                                Source Code
 0    pkg:/source/main.brs(12)                       msg = wait(0, m.port)
 1*   pkg:/source/promises.brs(264)           subpromises_internal_notifyListeners(event as object)
 2    ...RTA_OnDeviceComponentTask.brs(81)      message = wait(waitDelay, m.port)
chrisdp commented 1 year ago

@arturocuya I think I found the issue. It seems to be related to the fact that we do not unobserve the state. I am going to submit a fix to our internal project today just to make sure we don't see regressions and can put it forward here next week if it looks good.

Before: image

After: image