Skip to content

Broken SQL.open( data ) / SQL.exportData() #9

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

Closed
ltearno opened this issue Aug 29, 2012 · 12 comments
Closed

Broken SQL.open( data ) / SQL.exportData() #9

ltearno opened this issue Aug 29, 2012 · 12 comments
Assignees
Labels

Comments

@ltearno
Copy link

ltearno commented Aug 29, 2012

The mechanism through which it is possible to save and load a SQL instance seems to be broken.

The problem happens when opening a SQL instance passing the data returned by exportData() to the open(data) method.
To me it seems that although when created empty the database makes his buffer grow without problem, when the database is opened with a given uint8array, as the database grows the buffer is not reallocated and an overflow is generated, leading then to a corruption in the database disk image (SQLite exception: 11, database disk image is malformed)

Here is the test case proving the bug. The principle is simple, i first create an empty database, fill it with some records, then export its data, close it, reopen it with the exported data, and then going again with insertions.

Here is the Javascript code for this bug test-case, it is inpired by your demo page :

<script type="text/javascript" language="javascript">

// This variable will receive instance of SQLite
var db = null;

// method taken from your demo
function print(text) {
  var element = document.getElementById('output');
  element.innerHTML += "<br/>" + text;//.replace(/\n/g, '<br>');
}

// method taken from your demo
// only one modification : returns true when everything ok, and false when an exception has been triggered
function execute(commands) {
  try {
    print( "<b>" + commands + "</b>" );
    var data = db.exec(commands.replace(/\n/g, '; '));
    print(JSON.stringify(data, null, '  '));
    return true;
  } catch(e) {
    print(e);
    return false;
  }
}

var res = true;
for( i=0; i<10; i++ )
{
    print( "Pass " + i );

    var data = null;

    // if a SQLite instance has already been opened, we just export its data, 
    // close it and reopen a new instance with the exported data
    if( db != null )
    {
        data = db.exportData();
        db.close();
        db = null;

        print( "Exporting data and closing sqlite, bytes=" + data.length );
    }

    if( data != null )
    {
        db = SQL.open( data );
    }
    else
    {
        db = SQL.open();
        execute( "CREATE TABLE Persons( id INTEGER PRIMARY KEY AUTOINCREMENT, LastName varchar(255) NOT NULL );" );
    }

    execute( "SELECT count(*) as 'CountOfPersons' from Persons;" );

    for( j=0; j<100; j++ )
    {
        res = execute( "insert into Persons (LastName) values ('Toto-"+i+"-"+j+"');" );
        if( ! res )
            break;
    }

    if( ! res )
        break;
}

print( "Finished" );

</script>
lovasoa added a commit that referenced this issue Jun 12, 2014
Api & prepared statements
@acajic
Copy link

acajic commented Jul 30, 2014

Is it possible that this is still not fixed? I am having the same problem.

https://github.com/kripken/sql.js/issues/55

@acajic acajic mentioned this issue Jul 30, 2014
@lovasoa lovasoa self-assigned this Jul 30, 2014
@lovasoa
Copy link
Member

lovasoa commented Jul 30, 2014

This issue was opened a while ago, before I arrive in the project.
I'm now the only active maintainer of this repo. I'll take a look at this next weekend.

If you want to look at this yourself, I think the first thing to try is to close the database before exporting its contents. The method to change is here: https://github.com/kripken/sql.js/blob/master/coffee/api.coffee#L374

@acajic
Copy link

acajic commented Jul 30, 2014

Thank you for a quick response, I will look at the code right away. Will post if I have any progress.

@acajic
Copy link

acajic commented Aug 1, 2014

https://github.com/kripken/sql.js/blob/master/coffee/api.coffee#L374

is a one-line method:

'export': -> new Uint8Array FS.root.contents[@filename].contents

I am not familiar with coffeescript but it seems to me that the file by the name of @filename that is supposed to be created inside constructor does not get updated when I perform INSERT/UPDATE/DELETE commands agains db.

lovasoa added a commit that referenced this issue Aug 3, 2014
@lovasoa
Copy link
Member

lovasoa commented Aug 3, 2014

I'm sorry, but I can't reproduce this issue, nor #55. I added a test to the test directory, and created a jsfiddle with the code reported here (http://jsfiddle.net/3tx8G/). Both behave as expected.

Can you please provide a js file that triggers the error?

@acajic
Copy link

acajic commented Aug 19, 2014

I updated JSFiddle so that my problem is now apparent:

http://jsfiddle.net/3tx8G/3/

@lovasoa
Copy link
Member

lovasoa commented Aug 20, 2014

Ok, I'll look at it after my holidays. Thank you for the report, and for the test case.

@acajic
Copy link

acajic commented Sep 4, 2014

Are there any news on this issue?
Feel free to use the server that is used in jsFiddle, I hope it will stay functional in the following days...

@lovasoa
Copy link
Member

lovasoa commented Sep 4, 2014

I've added the sqlite db from your example to the tests.

The tests break now, you can see it here: https://travis-ci.org/kripken/sql.js

The fix I first thought of (closing the db before exporting its contents) doesn't work.

So, I really don't know where the problem is. I'm going to debug this, but it will require much more work than I first imagined.

I will investigate during one of the next weekend. If anyone wants to help, he's welcome, and I'll work with them

@acajic
Copy link

acajic commented Sep 4, 2014

OK, thanks for the update. I doubt that I can make a contribution but I will look into the code and try to make some sense out of it all.

@lovasoa
Copy link
Member

lovasoa commented Sep 4, 2014

The code really isn't very complex. It's just bindings to the original sqlite library, the documentation of which can be found here: http://www.sqlite.org/c3ref/funclist.html

For this particular bug, I think the best is to compile the whole thing in debug mode with emscripten (with -g) execute it in the browser, and add breakpoints in the emscripten virtual filesystem code to see what's going on.

@acajic
Copy link

acajic commented Sep 5, 2014

Database = (function() {
  function Database(data) {
    this.filename = 'dbfile_' + (0xffffffff * Math.random() >>> 0);
    ...

When testing test_issue55.js, this constructor gets called 2 times.
First time instantiating the database based on the 'issue55.db' file.
Second time using the bytes array generated by the function db.export().

I compared the contents of the 'data' argument to this constructor both times it is called, they are identical.
But this we already knew.

Then I inspected the db.export() function and it is a one-liner so I believe nothing can go wrong there.
So the problem can be:
1) db.export() is referencing the wrong file by using 'this.filename'.
2) When db.run() executes, the changes are manifested inside the 'this.db' object but not in the underlying file referenced by 'this.filename'.
3) (out of ideas) FS.readFile somehow caches the value of the file and does not return correct result if the file was modified in the meantime.

Pursuing 2), I checked db.run() function.
It contains an if-else blocks where an 'if' block delegates all the work to 'sqlite3_exec' function and the 'else' block depends on two functions - 'sqlite3_prepare_v2' and 'sqlite3_step'.
I tried to look for potential problem inside 'sqlite3_exec', 'sqlite3_prepare_v2' or 'sqlite3_step' functions, but they are too complicated for me.

Number 1) seems more likely to be the actual problem. But I have not yet made any progress in this direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants