donejs / bitballs

A basketball tourney application.
https://donejs.com/bitballs.html
MIT License
11 stars 17 forks source link

Race issue in bitballs #329

Open justinbmeyer opened 6 years ago

justinbmeyer commented 6 years ago

In bitballs, there's a <select> two-way bound to team-that-is-being-created's color:

<select value:bind="team.color" class="form-control" id="team-color">
            {{# each(availableColors) }}
                <option value="{{ . }}">{{ . }}</option>
            {{/ each }}
        </select>

There's a bug where

  1. can-connect is adding the newly created team to the list of teams.
  2. availableColors changes, which causes a mutation,
  3. Removing the newly created color from the list of options.
  4. which updates the team-that-is-being-created's color

This all happens before the team-that-is-being-created is replaced by a new Team:

        this.teamSavePromise = this.team.save(function(){
            self.team = new Team();
        });

This "race" condition changes because of zones. Without zones, the mutation happens after the promise is resolved. With zones, the mutation fires before the zone fires.

justinbmeyer commented 6 years ago

One solution is to simply create the new team right away as the old one is being saved. I wonder if there's a better one.

Essentially, the problem is that can-connect/real-time will update the list before the .save() promise is resolved.

justinbmeyer commented 6 years ago

as an aside, I improperly changed createTeam to:

    createTeam: function(ev){
        if (ev) {
            ev.preventDefault();
        }
        var self = this;
        if(!this.team.color){
            this.team.color = this.availableColors[0];
        }
        this.team.tournamentId = this.tournamentId;
        debugger;
        var team = this.team;
        this.team = null;
        this.teamSavePromise = this.team.save(function(){
            self.team = new Team();
        }, function(){
            debugger;
        });

    },

This did not produce any errors, as it was seemingly trapped by zones and not reported.

justinbmeyer commented 6 years ago
==== JS stack trace =========================================

Security context: 0x24c21ac28799 <JSObject>
    1: dataToArray(aka dataToArray) [evalmachine.<anonymous>:~29581] [pc=0xc16d317649e](this=0x2949ddc82311 <undefined>,data=0x3c095c7e9c69 <JSArray[2]>)
    3: intersection [evalmachine.<anonymous>:30155] [bytecode=0x1e65efdfc7c1 offset=23](this=0x3c095c7e9e31 <regenerate map = 0x3a255de50c79>,argument=0x3c095c7e9c49 <regenerate map = 0x3a255de50c79>)
    5: /* anonymous */ [evalmachine.<anonym...

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory
 1: node::Abort() [/Users/justin/.nvm/versions/node/v8.5.0/bin/node]
 2: node::FatalException(v8::Isolate*, v8::Local<v8::Value>, v8::Local<v8::Message>) [/Users/justin/.nvm/versions/node/v8.5.0/bin/node]
 3: v8::internal::V8::FatalProcessOutOfMemory(char const*, bool) [/Users/justin/.nvm/versions/node/v8.5.0/bin/node]
 4: v8::internal::Factory::NewUninitializedFixedArray(int) [/Users/justin/.nvm/versions/node/v8.5.0/bin/node]
 5: v8::internal::(anonymous namespace)::ElementsAccessorBase<v8::internal::(anonymous namespace)::FastPackedSmiElementsAccessor, v8::internal::(anonymous namespace)::ElementsKindTraits<(v8::internal::ElementsKind)0> >::GrowCapacity(v8::internal::Handle<v8::internal::JSObject>, unsigned int) [/Users/justin/.nvm/versions/node/v8.5.0/bin/node]
 6: v8::internal::Runtime_GrowArrayElements(int, v8::internal::Object**, v8::internal::Isolate*) [/Users/justin/.nvm/versions/node/v8.5.0/bin/node]
 7: 0xc16d0c046fd
 8: 0xc16d317649e
 9: 0xc16d0cfa912

we might also want to look at memory leaks

justinbmeyer commented 6 years ago
new MutationObserver(document)
.connect(function(){
    console.log("THRID without ZONES");
})

outerPromise = new Promise(function(resolve){

    var inner = new Promise(function(resolve){
        setTimeout(resolve,100)
    })
    inner.then(function(){
        console.log("FIRST ALWAYS")
        var div = document.createElement("div");

        document.body.appendChild(div);
    })

    return inner;
})

outerPromise.then(function(){
    console.log("SECOND without ZONES")
})

resolve = function(){
    if(I AM DISPATCHING OTHER PROMISE HANDLERS) {
        addMINE TO BE DISPATCHED( this._handlers )
    }
}