Improper clearing of objects in race conditions

[ograycode]

Problem

When doing multiple queries back-to-back, persistence will return the wrong object when using the .list(tx, callback) function. I fist discussed in the google group earlier today, https://groups.google.com/forum/?fromgroups=#!topic/persistencejs/wChS50a1JRY.

Fix

Call persistence.clean() before making the query to ensure proper clearing of objects and returning to the user what is expected.

Test

The test for this fix is housed inside of https://gist.github.com/3656872. With index.html containing existing code that demonstrates the problem, and index_fixed.html pointing to my cloned repo which demonstrates the fix. The console outputs the object._type.

Before Fix: The console output : https://gist.github.com/3656872#file_console_output.txt. Here, you can clearly see that the wrong objects are being returned.

After Fix: The console output: https://gist.github.com/3656872#file_console_output_fixed.txt. After the fix, you can see that the correct objects are being returned.

Git Commit Message

This fix is to ensure that PersistenceJS always returns only those objects that it has just queried, no matter how fast the queries are happening. By calling persistence.clean(); after we ensure we have a transaction, the following objects are cleared: this.trackedObjects this.objectsToRemove this.objectsRemoved this.globalPropertyListeners this.queryCollectionCache

[zefhemel]

This may fix this particular problem, but completely break the way persistence.js works. The idea is that once you persistence.add an object, its changes will be tracked forever and persisted on every flush. However, calling persistence.clean() all tracked objects are cleared, meaning no reference are held to them, and therefore changes made to those objects from that point onwards will no longer be persisted.

[ograycode]

@zefhemel thanks for the feedback, I'll see if I can find a different fix tonight or tomorrow.

The problem appears to be from line 696 of persistence.store.sql.js

var e = rowToEntity(session, entityName, r, mainPrefix);

rowToEntity (defined on ln. 336) is returning the wrong entity, whereas the entityName, r, and mainPrefix it is passing in look to be correct.

EDIT: Upon even further investigation, line 704 (session.add(e)) is also broken. The heart of the problem looks to be that the rows have the same primary key (id), even though they are on different tables. A good solution might be a hash based upon table name then id, instead of just id.

Looks to be a little more work than I expected, but worth it as it may prevent other issues down the road, and better supports pre-seeded data.

[zefhemel]

The ids of entities are assumed to be globally unique (cross entities even), by default UUIDs are used, do you fiddle with IDs somewhere maybe?

[ograycode]

Yes, the data is pre-seeded in the tables using manual insert statements and the ids are the same on a per-row basis across different tables, so row 1 of table 1 has the same id as row 1 of table 2. While I don't think pre-seeded data is something unreasonable, it is certainly not something that this library appears to fully support.

Instead of session.trackedObjects[id] it may be a better idea to have it be something along the lines of session.trackedObjects[entity_name][id].

In turn this would add better support for pre-seeded data, and help pave the way for user-defined primary keys.

[zefhemel]

Pre-seeded IDs are "officially" not supported at all, generated ids are holy, touch them at your own risk. You took that risk and bang ;) What you can do is make the ids unique yourself, by prepending them with the entity name yourself.

[ograycode]

Well I think this pull request had come to its end. So closing, I may try to branch and add better support as outlined above.