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

App keeps crashing after moving from cordova-sqlite-ext to this version [including emojis] #7

Closed
ThijsAmersfoort opened this issue Sep 8, 2016 · 16 comments

Comments

@ThijsAmersfoort
Copy link

ThijsAmersfoort commented Sep 8, 2016

Updated my app from cordova-sqlite-ext to this version, and on a physical HTC device the apps starts as normal, but as soon as html is displayed and app tries to get some tekst from the database, the ui hangs and the app crashes with the following error code:

Fatal signal 6 (SIGABRT), code -6 in tid 30707 (pool-2-thread-2)

This is after a few hundred red lines with warnings, errors and more, beginning with

DETECTED ERROR IN APPLICATION: input is not valid Modified UTF-8: illegal continuation byte 0x3f

Doing a little googling on this error, it appears that "There is a known NDK bug whereby GetStringUTFChars() incorrectly converts supplementary Unicode characters, producing an incorrect and invalid UTF-8 sequence. In my case, the resulting string was a JSON buffer. When the buffer was passed to the JSON parser, the parser promptly failed because one of the UTF-8 characters of the extracted UTF-8 had an invalid UTF-8 prefix byte." (http://stackoverflow.com/questions/12127817/android-ics-4-0-ndk-newstringutf-is-crashing-down-the-app) Indeed the app is trying to get a string of tekst from the database which contains special UTF8 characters.

In the simulator, the app does not crash, surprisingly, but is displaying the UTF8 characters incorrect. ó's are changed into ?ffffffc3??ffffffb3??ffffffc3??ffffffb3?, and • is turned into ?ffffffe2??ffffff80??ffffffa2? and so on.

@brodycj
Copy link
Contributor

brodycj commented Sep 8, 2016

Dag @ThijsAmersfoort,

Thanks for reporting this.

I suspect this would be caused by characters such as U+1F603 (SMILING MOUTH OPEN) ref: https://code.google.com/p/android/issues/detail?id=81341

Please report if I am mistaken here, or if you see any other characters that should be explicitly tested when I fix this issue.

SQLCipher for Android encountered a similar issue: sqlcipher/android-database-sqlcipher#199 which I reproduced in sqlcipher/sqlcipher-android-tests#10.

@ThijsAmersfoort
Copy link
Author

Looking at the error and the incorrect representation in the simulator, this indeed seems to be the case. The links states that the crash happens if the character starts with a value between 0xf0 and 0xf4, and my error is with 0x3f. This unfortunately means its an Android bug. Is there a way to fix this in the plugin, like 'convert to surrogates', as is mentioned? Otherwise, even when a future Android version supports it, apps won't run in earlier versions, or are forced to use the non-native solution.

@brodycj
Copy link
Contributor

brodycj commented Sep 8, 2016

I think there are multiple ways to fix this and will investigate. I am very reluctant to promise a timeline due to my current backlog. I will test on an Android 4.4.2 device which I expect to reproduce the issue.

@brodycj
Copy link
Contributor

brodycj commented Sep 8, 2016

Hey @ThijsAmersfoort,

The following test does not crash for me on my Android 4.4.2 device:

        it(suiteName + 'String emoji test:  \\u1F603 SMILING FACE (MOUTH OPEN)', function(done) {
          //if (isWP8) pending('...');

          var db = openDatabase("String-emoji-test.db", "1.0", "Demo", DEFAULT_SIZE);
          expect(db).toBeDefined();

          db.transaction(function(tx) {
            expect(tx).toBeDefined();

            tx.executeSql('SELECT UPPER(?) AS uppertext', ['Test\uD83D\uDE03'], function(tx, res) {
              expect(res.rows.item(0).uppertext).toBe('--'); // shows TEST with the emoji smile

              // Close (plugin only) & finish:
              (isWebSql) ? done() : db.close(done, done);
            });
          });
        }, MYTIMEOUT);

Can you share a simple piece of sample code to reproduce the crash?

@ThijsAmersfoort
Copy link
Author

ThijsAmersfoort commented Sep 8, 2016

I can. I was able to reproduce the problem in the standard cordova 'hello' app, with just this plugin added. Upon startup, after the device was ready, through the console I executed the following lines of code:

var db = window.sqlitePlugin.openDatabase({name: 'demo.db', location: 'default'});
db.executeSql('CREATE TABLE DemoTable (vid int(8), tekst varchar)',[],function(){
   console.log('succes')
},function(e){
   console.log(e)
});
db.executeSql('INSERT INTO DemoTable (vid,tekst) VALUES (?,?)',[49001004,'vóór'],function(){
},function(e){
   console.log(e)
});
db.executeSql('SELECT vid,tekst FROM DemoTable WHERE vid=49001004',[],function(res){
   var aant=res.rows.length;
   console.log(aant);
   for(var i=0;i<aant;i++)
      console.log(res.rows.item(i))
},function(e){
   console.log(e)
});

The table was created, the insert did work, but upon the select statement, the application crashed before a callback function could be called, and the same error was generated as earlier. Clearly the ó is the problematic character here. I ran the program on a HTC One running Android 5.0.2.

@brodycj
Copy link
Contributor

brodycj commented Sep 8, 2016

Can you do me a favor: try it with the following test and report if it succeeds or crashes on your device?

db.transaction(function(tx) {
  tx.executeSql('SELECT LOWER(?) AS lowertext', ['Vóór'], function(tx, res) {
    console.log('SUCCESS');
  });
});

The following test function, adapted from the existing test suite, runs OK on my Android 4.4.2 device:

        it(suiteName + 'EU String binding test', function(done) {
          var db = openDatabase("EU-string-binding-test.db", "1.0", "Demo", DEFAULT_SIZE);

          db.transaction(function(tx) {
            tx.executeSql('SELECT LOWER(?) AS lowertext', ['Vóór'], function(tx, res) {
              expect(res.rows.item(0).lowertext).toBe('vóór');

              // Close (plugin only) & finish:
              (isWebSql) ? done() : db.close(done, done);
            });
          });
        }, MYTIMEOUT);

I am suspicious that my device got an update that fixes the possible crash. I cannot promise when I would get a chance to try it with other Android device or simulator versions.

The StackOverflow link you sent is nice but I don't like it so much since it would add an extra encoding step and take away from the performance advantage.

A better solution may be to rework the code to use NewString/GetStringChars instead of NewStringUTF/GetStringUTFChars. Again I cannot promise when I will get a chance to try this.

You may also want to try the version at https://github.com/litehelpers/Cordova-sqlite-evplus-legacy-free. It solves the memory problem even if you use the androidDatabaseImplementation: 2 setting.

@ThijsAmersfoort
Copy link
Author

That piece of code does not crash the app. This, combined with my previous comment, suggest that the problem is not caused when these characters are sent towards the native database implementation. This is why the INSERT query in my previous comment did not crash the app. Only when the characters are inside the database, and are selected with a SELECT query, the code fails and the app fails. So the problem seems to by in getting the text out of the database, not getting it there.

@brodycj
Copy link
Contributor

brodycj commented Sep 8, 2016

Unfortunately I still cannot reproduce your problem on my Android 4.4.2 device. I really cannot promise when I will get a chance to continue to with this issue. Did you see my suggestion to try the version at https://github.com/litehelpers/Cordova-sqlite-evplus-legacy-free?

FYI here is how I tried to reproduce the crash:

I created new Cordova project using the following command:

cordova create aat

In the new project I added the plugin using the following command:

cordova plugin add cordova-sqlite-evcore-extbuild-free

When I try cordova plugin ls:

cordova-sqlite-evcore-extbuild-free 0.8.1 "Cordova sqlite storage - free enterprise version with Android performance/memory improvements for PhoneGap Build"

Here is the full content of www/js/index.js:

/*
 * Licensed to the Apache Software Foundation (ASF) under one
 * or more contributor license agreements.  See the NOTICE file
 * distributed with this work for additional information
 * regarding copyright ownership.  The ASF licenses this file
 * to you under the Apache License, Version 2.0 (the
 * "License"); you may not use this file except in compliance
 * with the License.  You may obtain a copy of the License at
 *
 * http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing,
 * software distributed under the License is distributed on an
 * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
 * KIND, either express or implied.  See the License for the
 * specific language governing permissions and limitations
 * under the License.
 */
var app = {
    // Application Constructor
    initialize: function() {
        this.bindEvents();
    },
    // Bind Event Listeners
    //
    // Bind any events that are required on startup. Common events are:
    // 'load', 'deviceready', 'offline', and 'online'.
    bindEvents: function() {
        document.addEventListener('deviceready', this.onDeviceReady, false);
    },
    // deviceready Event Handler
    //
    // The scope of 'this' is the event. In order to call the 'receivedEvent'
    // function, we must explicitly call 'app.receivedEvent(...);'
    onDeviceReady: function() {
        app.receivedEvent('deviceready');
        var db = window.sqlitePlugin.openDatabase({name: 'demo.db', location: 'default'});

        db.executeSql('CREATE TABLE DemoTable (vid int(8), tekst varchar)',[],function(){
           console.log('succes')
        },function(e){
           console.log(e)
        });
        db.executeSql('INSERT INTO DemoTable (vid,tekst) VALUES (?,?)',[49001004,'vóór'],function(){
          // Brody extra log:
          console.log('INSERT OK');
        },function(e){
           console.log(e)
        });
        db.executeSql('SELECT vid,tekst FROM DemoTable WHERE vid=49001004',[],function(res){
           var aant=res.rows.length;
           console.log(aant);
           for(var i=0;i<aant;i++)
              console.log(res.rows.item(i))
            // Brody alert added:
            alert('SUCCESS with rows length: ' + aant);
        },function(e){
           console.log(e)
        });
    },
    // Update DOM on a Received Event
    receivedEvent: function(id) {
        var parentElement = document.getElementById(id);
        var listeningElement = parentElement.querySelector('.listening');
        var receivedElement = parentElement.querySelector('.received');

        listeningElement.setAttribute('style', 'display:none;');
        receivedElement.setAttribute('style', 'display:block;');

        console.log('Received Event: ' + id);
    }
};

app.initialize();

Basically I pasted your code into the generated index.js file, using the Atom editor to indent it with the rest, added an extra console.log line marked with a "Brody extra log:" comment, and added an extra alert statement marked with a "Brody alert added:" comment. When I run it on my Android 4.4.2 device I get the SUCCESS alert.

@ThijsAmersfoort
Copy link
Author

Thanks anyway. Unfortunately, this makes this version unusable for me now. I'll take a look at your suggestion and keep an eye on this thread to see if something changed.

@filimo
Copy link

filimo commented Aug 4, 2017

I had same problem with --debug only. There isn't the problem with --release parameter.

@brodycj
Copy link
Contributor

brodycj commented Jan 15, 2018

My apologies for the delay in general. I do see crash in certain cases of 4-octet emoji characters on Samsung Galaxy J3 test device with Android 5.1.1 and Motorola Moto E4 Plus test device with Android 7.1.1. I hope to resolve this within the near future.

Accepted solution to https://stackoverflow.com/questions/12127817/android-ics-4-0-ndk-newstringutf-is-crashing-down-the-app is to avoid NewStringUTF8(), thanks @ThijsAmersfoort for the link.

@brodycj brodycj changed the title App keeps crashing after moving from cordova-sqlite-ext to this version App keeps crashing after moving from cordova-sqlite-ext to this version [including emojis] Jan 17, 2018
@brodycj
Copy link
Contributor

brodycj commented Jan 17, 2018

I have only reproduced this issue in case of 4-byte emoji characters, updated the title to reflect this. Related issues:

I expect to fix this issue in the next major release (related to next major release of cordova-sqlite-storage discussed in storesafe/cordova-sqlite-storage#687)

brodycj pushed a commit to storesafe/android-sqlite-evcore-ndk-driver-free that referenced this issue Oct 11, 2018
brodycj pushed a commit to brodycj/cordova-sqlite-evcore-free-dependencies that referenced this issue Oct 12, 2018
brodycj pushed a commit to brodycj/cordova-sqlite-evcore-free-dependencies that referenced this issue Oct 18, 2018
for issue with 4-byte UTF-8 characters

ref: storesafe/cordova-sqlite-evcore-extbuild-free#7

(cordova-sqlite-evcore-free-dependencies 0.8.6)
brodycj pushed a commit that referenced this issue Oct 21, 2018
for 4-byte UTF-8 characters

and crash bug fix for Samaritan characters

from cordova-sqlite-evcore-free-dependencies 0.8.6

ref:
- #7
- #37

(cordova-sqlite-evcore-extbuild-free 0.9.9-rc1)
@brodycj
Copy link
Contributor

brodycj commented Nov 5, 2018

Now resolved with a workaround solution

brodycj pushed a commit that referenced this issue Nov 6, 2018
in cordova-sqlite-evcore-common-free version branch

with workaround for 4-byte UTF-8 crash bug ref:
- #44
- #7

and crash bug fix for Samaritan characters ref:
- #37

(cordova-sqlite-evcore-common-free 0.0.4)
brodycj pushed a commit to storesafe/android-sqlite-evcore-ndk-driver-free that referenced this issue Dec 17, 2018
Store & retrieve all 4-byte UTF-8 characters as single '?',
with no extra garbage characters

ref: storesafe/cordova-sqlite-evcore-extbuild-free#7
@ThijsAmersfoort
Copy link
Author

I revisited this plugin for my project, and now successfully managed to replace sqlite-plugin-ext by this plugin. The problem seems to be fixed indeed, on both my phone and older android versions in the emulator. Thanks for the fixing.

@brodycj
Copy link
Contributor

brodycj commented Mar 11, 2019

Thanks for confirming.

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

No branches or pull requests

3 participants