Skip to content

Commit

Permalink
Avoid an indefinite recursion that grows the call stack when reportin…
Browse files Browse the repository at this point in the history
…g the current state fails

We used to have a recursion based on Promises and Promise.delay, which caused the promise never to resolve
so eventually the stack would be exhausted.

This fixes it by using a simpler way to check if reporting the state is in progress and using a setImmediate to
call applyState outside of the Promise chain.

Change-Type: patch
Signed-off-by: Pablo Carranza Velez <pablo@resin.io>
  • Loading branch information
Pablo Carranza Velez committed Jul 26, 2017
1 parent 8e1a38d commit 215cfc0
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions src/device.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ exports.getDeviceType = memoizePromise ->
do ->
APPLY_STATE_SUCCESS_DELAY = 1000
APPLY_STATE_RETRY_DELAY = 5000
applyPromise = Promise.resolve()
applyPending = false
targetState = {}
actualState = {}
updateState = { update_pending: false, update_failed: false, update_downloaded: false }
Expand All @@ -176,8 +176,10 @@ do ->
applyState = ->
stateDiff = getStateDiff()
if _.size(stateDiff) is 0
applyPending = false
return
applyPromise = Promise.join(
applyPending = true
Promise.join(
utils.getConfig('apiKey')
device.getID()
(apiKey, deviceID) ->
Expand All @@ -202,7 +204,7 @@ do ->
Promise.delay(APPLY_STATE_RETRY_DELAY)
.finally ->
# Check if any more state diffs have appeared whilst we've been processing this update.
applyState()
setImmediate(applyState)

exports.setUpdateState = (value) ->
_.merge(updateState, value)
Expand All @@ -221,7 +223,7 @@ do ->
_.merge(targetState, updatedState)

# Only trigger applying state if an apply isn't already in progress.
if !applyPromise.isPending()
if !applyPending
applyState()
return

Expand Down

0 comments on commit 215cfc0

Please sign in to comment.