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

Prevent infinite recursion on unbindAndTeardown when Observe's _bindings is undefined and Observe is self-referential #461

Merged
merged 1 commit into from Sep 19, 2013

Conversation

bmomberger-reciprocity
Copy link
Contributor

Previously in util/bind/bind.js

                this._bindings--;
                // If there are no longer any bindings and
                // there is a bindteardown method, call it.
                if(!this._bindings){

If _bindings was undefined, this sets it to NaN and then does the teardown. If in the teardown this object is touched again, having NaN for bindings will decrement to NaN again, still be falsy, and run the teardown again (eventually leading to a stack fault).

The code in this pull request sets _bindings to 0 if it is undefined. Any subsequent visit to unbindAndTeardown will then decrement it to -1 or less, which is okay because we've already started the teardown process.

…bindings is always a number when incremented/decremented
@justinbmeyer
Copy link
Contributor

Thanks brad!

Justin Meyer
847-924-6039

On Fri, Aug 16, 2013 at 3:03 PM, Brad (Bradley) Momberger <
notifications@github.com> wrote:

Previously in util/bind/bind.js

            this._bindings--;
            // If there are no longer any bindings and
            // there is a bindteardown method, call it.
            if(!this._bindings){

If _bindings was undefined, this sets it to NaN and then does the
teardown. If in the teardown this object is touched again, having NaN for
bindings will decrement to NaN again, still be falsy, and run the teardown
again (eventually leading to a stack fault).

The code in this pull request sets _bindings to 0 if it is undefined. Any
subsequent visit to unbindAndTeardown will then decrement it to -1 or less,

which is okay because we've already started the teardown process.

You can merge this Pull Request by running

git pull https://github.com/bmomberger-reciprocity/canjs self-limiting-unbind

Or view, comment on, or merge it at:

#461
Commit Summary

  • Preventing infinite recursion on unbindAndTeardown by ensuring that
    _bindings is always a number when incremented/decremented

File Changes

Patch Links:

@daffl
Copy link
Contributor

daffl commented Aug 30, 2013

Looks good to me. What was the case when this is happening? Should we add a test for it?

@airhadoken
Copy link
Contributor

It was happening in /reciprocity/ggrc-core until we applied the fix, but I am having trouble distilling the problem down into a simple fiddle. I think it might be in part due to us overriding can.Model.model.

@daffl
Copy link
Contributor

daffl commented Sep 19, 2013

Ok. Merging this in since it doesn't seem to break any of the existing tests.

daffl added a commit that referenced this pull request Sep 19, 2013
Prevent infinite recursion on unbindAndTeardown when Observe's _bindings is undefined and Observe is self-referential
@daffl daffl merged commit 7725939 into canjs:master Sep 19, 2013
@justinbmeyer
Copy link
Contributor

I bet calling unbind without calling bind might do it.

Sent from my iPhone

On Sep 19, 2013, at 4:24 PM, David Luecke notifications@github.com wrote:

Ok. Merging this in since it doesn't seem to break any of the existing tests.


Reply to this email directly or view it on GitHub.

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 this pull request may close these issues.

None yet

4 participants