Page 1 of 3 123 LastLast
Results 1 to 10 of 28

Thread: Async proxy race condition

    Looks like we can't reproduce the issue or there's a problem in the test case provided.
  1. #1
    Sencha Premium Member robboerman's Avatar
    Join Date
    Nov 2007
    Location
    Pijnacker, the Netherlands
    Posts
    94
    Vote Rating
    13
      0  

    Default Async proxy race condition

    Hi guys,

    We just discovered a bug when using a proxy that asynchronously writes data, say a SQL proxy. We have a store that uses a asynchronous proxy and have set autoSync to true. This means that when we add a new record to the store it is immediately synced with the proxy. When the proxy sync starts it selects all phantom records (without an id) and tries to sync them to storage. With a remote proxy this can take some time. When a new record is added to the store during that time a new proxy sync is started. That sync again selects all 'phantom' records and tries to store them. Because the id of the first record has not been created yet (at the end of the proxy sync) it is treated as a new phantom record again. The end result is that records get stored by the proxy more than once. I think that a record should be flagged by the proxy when it tries to send it off so that a new proxy sync does not select it again.

    The usecase is easy to reproduce. Just go to the Sencha Touch Kitchen sync (http://cdn.sencha.io/touch/sencha-to...ink/index.html) and dump the below code in the developer console.

    Code:
    Ext.define('User', {
        extend: 'Ext.data.Model',
    
        config: {
            fields: ['id', 'name', 'age', 'gender']
        }
    });
    
    // Uses the User Model's Proxy
    var store = Ext.create('Ext.data.Store', {
        model: 'User',
        autoSync: true,
        proxy: {
        	type: 'sql'
        }
    });
    
    
    store.add({name: "Rob", age: "36", gender: "male"});
    store.add({name: "Robin", age: "33", gender: "female"});
    store.add({name: "Robinhood", age: "250", gender: "male"});
    The result is:

    - add record 1
    - sync sends record 1
    - add record 2
    - sync sends record 1
    - sync sends record 2
    - add record 3
    - sync sends record 1
    - sync sends record 2
    - sync sends record 3
    - sync 1 ends, created record 1 in SQL
    - sync 2 ends, created record 1 and 2 in SQL
    - sync 3 ends, created record 1, 2 and 3 in SQL

    Leaving you with 6 records instead of 3:



    Screen Shot 2013-07-25 at 1.20.56 PM.png
    Rob Boerman

    2Gears.com
    Delft, the Netherlands
    Blog: 2gears.com/blog

  2. #2
    Sencha Premium User mitchellsimoens's Avatar
    Join Date
    Mar 2007
    Location
    Gainesville, FL
    Posts
    40,382
    Vote Rating
    1536
      0  

    Default

    Thanks for the report! I have opened a bug in our bug tracker.

  3. #3
    Sencha User
    Join Date
    Jan 2011
    Posts
    134
    Vote Rating
    5
      0  

    Default Would this be a work around?

    Would turing off autosync be a work around?

  4. #4
    Sencha Premium Member
    Join Date
    Jul 2012
    Posts
    98
    Vote Rating
    3
      1  

    Default

    Absolutely not. I've come across this same issue in our project and it caused weeks of frustrations and hardly reproducible test cases. Finally I dig into the sencha source code and found the same conclusions: the phantom flag is not handled properly, at least it's not enough the way it's handled.

    You don't need autoSync to reproduce this error, you can simply call the store's sync method twice in a row, like this:
    store.add({field: "test"});
    store.sync();
    store.sync();

    This causes the one added record to be added twice by the proxy.

    You could ask why I'd call sync twice in a row - well I don't, but this might be connected to a user event, to create a record and sync the store when tapped on something:
    store.add({field:"test"});
    store.sync();

    Now this might be triggered twice by the user, the second coming quickly after the first:
    store.add({field:"test"});
    store.sync();
    [very little delay]
    store.add({field:"test2"});
    store.sync(); //

    At the last sync call, the first record will be added again, because the first sync didn't return yet, so the firstly added record's phantom status is still true. So the result will be: "test1" added twice, "test2" added once.

    This is just one real use-case, generally there can be a lot. I think this is something not the application developer should take care of, but rather the sencha sdk should work more reliably regarding this.

    Also note that this is not a simple typo bug, or small issue, but rather a logical error in how the sync mechanism work, so I imagine to fix this properly would be not an easy issue. Although it's strange why this error still wasn't reported more frequently, I myself reported it once here in the forums, once through the private sencha support, but no actions has been taken care of yet.

    What we ended up doing, to make our app usable was that we've overridden the default sync method like this:
    - introduced a new "_blocked" flag, which turns true after calling sync
    - when calling a sync and _blocked is not true, we just call the original sencha sync method
    - when calling a sync, while _blocked is true, the original sencha sync method is not called, but a new "_syncCalled" flag is set to true to detect that a sync was called during a "blocked" status, so when the currently running (original) sync method returns (by that I mean that the 'write' event returns, the sync call itself returns immediately of course), the sync method will have to be called again

    This basically solves this record duplication issue, but it's not a perfect solution, for example:
    store.add({field:"hey"});
    store.sync();
    store.add({field:"hey2"});
    store.on('write', ...);
    store.sync();

    Here, the problem is that the write event listener will return for the originally called sync that only has the results for the record added first. So if you have auto id-generation, where you only know the id once the 'write' completes after adding a record and syncing, you can have a problem with this: when the 'write' returns it will only have the result for the first record, not the second, so the second record's id is still not the proper id you're looking for.

    Note that the write event can have other problems as well, for example if you rely on it return at one point after calling sync, it may never due - for example, if you supply some data that's not valid, so the store will not even initiate and batches by it's proxy.

    I can share the override code that we came up with, but again it has a flaw as you can read above.
    Now that I think of it, having a fix for that flaw is not that difficult, I just didn't have too much time for it at the time, and the main problem was fixed anyway. But would there be interest to fix that ? If so I would consider doing that.

  5. #5
    Sencha Premium Member
    Join Date
    Jul 2012
    Posts
    98
    Vote Rating
    3
      0  

    Default Remove issue as well

    Also, another issue is connected with this one:
    When a sync is in progress and you remove a record from a store which still has a phantom flag, then the remove will do nothing (it assumes the record doesn't exist - which is true at the moment, but not any more when the 'write' completes).

    A scenario:
    var rec = store.add({field:"test"})[0]; // rec.phantom === true

    store.sync();
    // now a sync process starts, it doesn't return yet!
    // we call remove here before the write returns
    store.remove(rec); // at this point, because rec.phantom is still true, nothing happens
    store.sync();
    // 'write' returns only here, sets the phantom to false

    So what happens from the user's point of view is that he created a record, removed it, but it wasn't removed, it's still in the sencha store and the local database as well (if using an sql proxy for example).

  6. #6
    Sencha User
    Join Date
    Sep 2013
    Posts
    2
    Vote Rating
    0
      0  

    Default

    Hi,

    I'm having the same issues that you are describing in this thread. I'm using WebSQL to store some local data that needs to be uploaded to a server when possible (i.e. there's internet connection available). At first I was using shepsii's proxy and thought that was the source of the problem. Since then, I've upgraded my App to Sencha Touch 2.2 and have begun using the "official" sqlite proxy but the problem persists (although using uuid's as identifiers for my models seems to alleviate the problem).

    This thread helps me to confirm my beliefs about the asynchronous nature of the sync() call causing race conditions and thus duplicated records and inconsistencies. I would be very grateful If you could send me the override that fixed the problem for you. This duplicated records issue is giving me a lot of headaches and I've already spent countless hours trying to fix it without success. I've tried coding my own override but I can't get it to work (this is my first ST2 project and I'm not experienced enough tinkering with the class system).

    Thanks in advance

    Quote Originally Posted by grizzly21 View Post
    Absolutely not. I've come across this same issue in our project and it caused weeks of frustrations and hardly reproducible test cases. Finally I dig into the sencha source code and found the same conclusions: the phantom flag is not handled properly, at least it's not enough the way it's handled.

    You don't need autoSync to reproduce this error, you can simply call the store's sync method twice in a row, like this:
    store.add({field: "test"});
    store.sync();
    store.sync();

    This causes the one added record to be added twice by the proxy.

    You could ask why I'd call sync twice in a row - well I don't, but this might be connected to a user event, to create a record and sync the store when tapped on something:
    store.add({field:"test"});
    store.sync();

    Now this might be triggered twice by the user, the second coming quickly after the first:
    store.add({field:"test"});
    store.sync();
    [very little delay]
    store.add({field:"test2"});
    store.sync(); //

    At the last sync call, the first record will be added again, because the first sync didn't return yet, so the firstly added record's phantom status is still true. So the result will be: "test1" added twice, "test2" added once.

    This is just one real use-case, generally there can be a lot. I think this is something not the application developer should take care of, but rather the sencha sdk should work more reliably regarding this.

    Also note that this is not a simple typo bug, or small issue, but rather a logical error in how the sync mechanism work, so I imagine to fix this properly would be not an easy issue. Although it's strange why this error still wasn't reported more frequently, I myself reported it once here in the forums, once through the private sencha support, but no actions has been taken care of yet.

    What we ended up doing, to make our app usable was that we've overridden the default sync method like this:
    - introduced a new "_blocked" flag, which turns true after calling sync
    - when calling a sync and _blocked is not true, we just call the original sencha sync method
    - when calling a sync, while _blocked is true, the original sencha sync method is not called, but a new "_syncCalled" flag is set to true to detect that a sync was called during a "blocked" status, so when the currently running (original) sync method returns (by that I mean that the 'write' event returns, the sync call itself returns immediately of course), the sync method will have to be called again

    This basically solves this record duplication issue, but it's not a perfect solution, for example:
    store.add({field:"hey"});
    store.sync();
    store.add({field:"hey2"});
    store.on('write', ...);
    store.sync();

    Here, the problem is that the write event listener will return for the originally called sync that only has the results for the record added first. So if you have auto id-generation, where you only know the id once the 'write' completes after adding a record and syncing, you can have a problem with this: when the 'write' returns it will only have the result for the first record, not the second, so the second record's id is still not the proper id you're looking for.

    Note that the write event can have other problems as well, for example if you rely on it return at one point after calling sync, it may never due - for example, if you supply some data that's not valid, so the store will not even initiate and batches by it's proxy.

    I can share the override code that we came up with, but again it has a flaw as you can read above.
    Now that I think of it, having a fix for that flaw is not that difficult, I just didn't have too much time for it at the time, and the main problem was fixed anyway. But would there be interest to fix that ? If so I would consider doing that.

  7. #7
    Sencha Premium Member
    Join Date
    Jul 2012
    Posts
    98
    Vote Rating
    3
      0  

    Default Quick fix to prevent duplication (not a real proper fix though)

    My solution is only a dirty quick hack on preventing the duplicate record creation issue, but it's not perfect (as you can read above), for example the write event is not triggered properly so if you rely on "write" to receive the success of the operation it could cause problems.

    If you just simply want to prevent duplication the easiest thing is to set record.phantom = false after calling sync on the store.

    But what I use so far is this, let me know if it helps for you:



    Code:
    // General solution to solve 'multiple sync call issue' :
    // that is a problem when calling sync on the same store multiple times,
    // before the previous asynchronous sync() call was completed ('write' event).
    // This makes sure sync is not called on a store before a previous sync has completed,
    // but it will call another sync when the previous one completed.
    
    
    Ext.data.Store.prototype.originalSyncMethod = Ext.data.Store.prototype.sync;
    
    
    // Makes sure to only call the 'real' sync if it's not in the process of a 
    // previous call.
    // It delays the seconds call to start after the first finishes.
    //
    // This has sideeffects though:
    // 
    // Example:
    // store.remove(item1)
    // store.sync()
    // store.remove(item2)
    // store.sync()
    // otherStore.remove(otherItem)
    // otherStore.sync()
    // Here, the order the items will be deleted is:
    // item1, otherItem, item2 (because item2 is blocked till item1 returns,
    // but otherItem is not blocked since it belongs to a different store).
    
    
    Ext.data.Store.prototype.sync = function() {
    
    
        if (this.__storeIsBlocked) {
    
    
            // console.log('store is blocked', this.getStoreId());
    
    
            this.__storeNeedsSync = true;
    
    
            return {
                added: [],
                updated: [],
                removed: []
            };
    
    
        } else {
    
    
            this.__storeIsBlocked = true;
    
    
            // problem was: onWriteComplete runs first (blocked)
            // onStoreWrite runs second (__storeIsBlocked will be set to false only here)
            // -> you can't do nested syncWithListeners properly (nested will return as blocked)
            // so we change the order of listeners:
            // either A:
            //this.addBeforeListener('write', StoreUtils.onStoreWrite, this);
            //var syncRes = this.originalSyncMethod();
            // or B:
    
    
            var syncRes = this.syncWithListener(StoreUtils.onStoreWrite, this.originalSyncMethod, 'before'); // orig
            
            return syncRes;
        }
    };
    
    
    //
    // Helper method to call sync on a store and invoke a callback when the write event triggers (or there's nothing to write)
    //
    
    
    Ext.data.Store.prototype.syncWithListener = function(onWriteComplete, syncMethod, order) {
    
    
        this.on('write', onWriteComplete, this, {single:true}, order || undefined);
    
    
        var syncResult = syncMethod ? syncMethod.apply(this) : this.sync();
    
    
        if (syncResult.added.length === 0 &&
        syncResult.updated.length === 0 &&
        syncResult.removed.length === 0) {
    
    
            this.removeListener('write', onWriteComplete, this, {single:true});
            onWriteComplete(this);    
        }
    
    
        return syncResult;
    };

  8. #8
    Sencha Premium Member robboerman's Avatar
    Join Date
    Nov 2007
    Location
    Pijnacker, the Netherlands
    Posts
    94
    Vote Rating
    13
      0  

    Default

    Hi, for overriding it's better to use the proper Ext.define('my.fix',{override: 'Ext.Foo.Bar',... }) syntax instead of manually overriding prototype functions. Much less error prone, easier to call the overridden function and easier to remove when the fix has been added to ExtJS/Touch

    Cheers,
    Rob
    Rob Boerman

    2Gears.com
    Delft, the Netherlands
    Blog: 2gears.com/blog

  9. #9
    Sencha User
    Join Date
    Sep 2013
    Posts
    2
    Vote Rating
    0
      0  

    Default

    Hi,

    thanks for you answers, really appreciate it . grizzly21, I've tried your code and I think it really solves the problem for me so thank you very much!. I'm doing multiple syncs() on the store whenever I add a new record and so far none of the tests have produced a duplicated record. Oh, by the way, I think there's a missing piece of code in your proposed fix. You pass the function StoreUtils.onStoreWrite as an argument to syncWithListener but you didn't include that in your snippet. I've added a little function to replace StoreUtils.onStoreWrite that looks like this:

    Code:
    fnOnStoreWrite = function(store)
            {
                //Clear the storeIsBlocked flag as we're done sync()ing
                store.__storeIsBlocked = false;
    
                //If store's sync() was called during the previous sync(), call sync() again.
                if(store.__storeNeedsSync)
                {
                    store.__storeNeedsSync = false;
                    var syncRes = store.sync();
                }
            }
    I don't know If I'm missing something but that just seems to work. I'm aware of the limitations of this hack but it's fine for my use case. I'm not really using the "onWrite" event for anything and I don't really care about what id gets generated for the record, as long as it's saved properly on WebSQL without duplications.

    Thanks for your help,
    Millen

  10. #10
    Sencha Premium Member
    Join Date
    Jul 2012
    Posts
    98
    Vote Rating
    3
      0  

    Default

    thats true override is probably a bit better but essentially it should work the same way

Page 1 of 3 123 LastLast

Tags for this Thread

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •