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

FontContext::fetch segfaults in Android in Scene Update #978

Closed
hallahan opened this issue Sep 16, 2016 · 3 comments
Closed

FontContext::fetch segfaults in Android in Scene Update #978

hallahan opened this issue Sep 16, 2016 · 3 comments
Assignees

Comments

@hallahan
Copy link
Contributor

While working on MBTiles Support (#960), I've found that I can very nicely add an MBTiles file to a data source dynamically using the scene update mechanism. This works great on OS X, but it segfaults on Android.

After doing a bit of debugging, I've pinpointed that this is when we re-fetch a font if it has been specified in the scene.yaml. In specific, it happens right when we do the startUrlRequest call. It makes sense that this happens only one one platform, since this is platform specific code.

void FontContext::fetch(const FontDescription& _ft) {

    const static std::regex regex("^(http|https):/");
    std::smatch match;

    if (std::regex_search(_ft.uri, match, regex)) {
        // Load remote
        resourceLoad++;
        startUrlRequest(_ft.uri, [&, _ft](std::vector<char>&& rawData) {
            if (rawData.size() == 0) {
                LOGE("Bad URL request for font %s at URL %s", _ft.alias.c_str(), _ft.uri.c_str());
            } else {
                std::lock_guard<std::mutex> lock(m_mutex);
                char* data = reinterpret_cast<char*>(rawData.data());

                for (int i = 0, size = BASE_SIZE; i < MAX_STEPS; i++, size += STEP_SIZE) {
                    auto font = m_alfons.getFont(_ft.alias, size);
                    font->addFace(m_alfons.addFontFace(alfons::InputSource(data, rawData.size()), size));
                }
            }

            resourceLoad--;
        });
    } else {
    ...

If I comment out the font block and the references to it in the scene.yaml, my scene update activating MBTiles works.

@hallahan
Copy link
Contributor Author

In specific, I did some log messages in Android's startUrlRequest.

bool startUrlRequest(const std::string& _url, UrlCallback _callback) {
    LOG("android native startUrlRequest _url: %s", _url.c_str());
    jstring jUrl = jniRenderThreadEnv->NewStringUTF(_url.c_str());
    LOG("jstring jUrl = jniRenderThreadEnv->NewStringUTF(_url.c_str());");
    // This is probably super dangerous. In order to pass a reference to our callback we have to convert it
    // to a Java type. We allocate a new callback object and then reinterpret the pointer to it as a Java long.
    // In Java, we associate this long with the current network request and pass it back to native code when
    // the request completes (either in onUrlSuccess or onUrlFailure), reinterpret the long back into a
    // pointer, call the callback function if the request succeeded, and delete the heap-allocated UrlCallback
    // to make sure nothing is leaked.
    jlong jCallbackPtr = reinterpret_cast<jlong>(new UrlCallback(_callback));
    LOG("jlong jCallbackPtr = reinterpret_cast<jlong>(new UrlCallback(_callback));");
    jboolean methodResult = jniRenderThreadEnv->CallBooleanMethod(tangramInstance, startUrlRequestMID, jUrl, jCallbackPtr);
    LOG("jboolean methodResult = jniRenderThreadEnv->CallBooleanMethod(tangramInstance, startUrlRequestMID, jUrl, jCallbackPtr);");
    return methodResult;
}

This is the corresponding log:

09-16 13:23:10.520 15091-15185/com.mapzen.tangram.android D/Tangram: TANGRAM platform_android.cpp:264: android native startUrlRequest _url: https://fonts.gstatic.com/s/montserrat/v7/zhcz-_WihjSQC0oHJ9TCYL3hpw3pgy2gAi-Ip7WPMi0.woff
09-16 13:23:10.520 15091-15185/com.mapzen.tangram.android A/libc: Fatal signal 11 (SIGSEGV), code 1, fault addr 0x2 in tid 15185 (AsyncTask #1)

What's odd is that it looks like that string transformation doesn't happen. But, that might just be the log not up to date with execution?

The long warning in the comments is suggesting to me something is going wrong here.

@hallahan
Copy link
Contributor Author

hallahan commented Dec 2, 2016

Looking a bit at this problem, we were noticing that jniRenderThreadEnv->NewStringUTF was not working, so we should check for exceptions from the JNI environment.

Here's an example:

http://stackoverflow.com/questions/2054598/how-to-catch-jni-java-exception

jclass c = env->FindClass("class/does/not/Exist");
if (env->ExceptionCheck()) {
  return;
}
// otherwise do something with 'c'...

@tallytalwar
Copy link
Member

Just noticed this happening on master:

Stacktrace:

12-14 15:41:59.340 19155 19155 D Tangram : TANGRAM tangram.cpp:270: Applying 1 scene updates
12-14 15:41:59.340 19155 19174 F libc    : Fatal signal 11 (SIGSEGV), code 1, fault addr 0x2 in tid 19174 (AsyncTask #1)
12-14 15:41:59.360  1391  2649 D SecContentProvider2: query(), uri = 15 selection = getToastGravityEnabledState
12-14 15:41:59.390  1391  2175 D SecContentProvider2: query(), uri = 15 selection = getToastEnabledState
12-14 15:41:59.390  1391  2657 D SecContentProvider2: query(), uri = 15 selection = getToastShowPackageNameState
12-14 15:41:59.420 19155 19155 D SecWifiDisplayUtil: Metadata value : SecSettings2
12-14 15:41:59.450   976   976 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
12-14 15:41:59.450   976   976 F DEBUG   : Build fingerprint: 'samsung/hero2qlteuc/hero2qlteatt:6.0.1/MMB29M/G935AUCS4APK1:user/release-keys'
12-14 15:41:59.450   976   976 F DEBUG   : Revision: '15'
12-14 15:41:59.450   976   976 F DEBUG   : ABI: 'arm'
12-14 15:41:59.450   976   976 F DEBUG   : pid: 19155, tid: 19174, name: AsyncTask #1  >>> com.mapzen.tangram.android <<<
12-14 15:41:59.450   976   976 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x2
12-14 15:41:59.490   976   976 F DEBUG   :     r0 00000000  r1 00000001  r2 eeb959b8  r3 eeb95930
12-14 15:41:59.490   976   976 F DEBUG   :     r4 f47bf800  r5 f74afec0  r6 d70ea6c0  r7 f47bc910
12-14 15:41:59.490   976   976 F DEBUG   :     r8 f4773904  r9 da0b18e0  sl eeb95174  fp eeb95150
12-14 15:41:59.490   976   976 F DEBUG   :     ip eeb95a18  sp eeb94e70  lr f45be733  pc f45be732  cpsr 40070030
12-14 15:41:59.490   976   976 F DEBUG   : 
12-14 15:41:59.490   976   976 F DEBUG   : backtrace:
12-14 15:41:59.490   976   976 F DEBUG   :     #00 pc 0024f732  /system/lib/libart.so (_ZN3art9JavaVMExt8JniAbortEPKcS2_+57)
12-14 15:41:59.490   976   976 F DEBUG   :     #01 pc 0025008f  /system/lib/libart.so (_ZN3art9JavaVMExt9JniAbortVEPKcS2_St9__va_list+54)
12-14 15:41:59.490   976   976 F DEBUG   :     #02 pc 000fc10b  /system/lib/libart.so (_ZN3art11ScopedCheck6AbortFEPKcz+30)
12-14 15:41:59.490   976   976 F DEBUG   :     #03 pc 0010006b  /system/lib/libart.so (_ZN3art11ScopedCheck5CheckERNS_18ScopedObjectAccessEbPKcPNS_12JniValueTypeE.constprop.95+1090)
12-14 15:41:59.490   976   976 F DEBUG   :     #04 pc 0010883f  /system/lib/libart.so (_ZN3art8CheckJNI12NewStringUTFEP7_JNIEnvPKc+374)
12-14 15:41:59.490   976   976 F DEBUG   :     #05 pc 000a083b  /data/app/com.mapzen.tangram.android-1/lib/arm/libtangram.so (_Z15startUrlRequestRKNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEENS_8functionIFvONS_6vectorIcS4_EEEEE+38)
12-14 15:41:59.490   976   976 F DEBUG   :     #06 pc 000d391d  /data/app/com.mapzen.tangram.android-1/lib/arm/libtangram.so (_ZN7Tangram19loadFontDescriptionERKN4YAML4NodeERKNSt3__112basic_stringIcNS4_11char_traitsIcEENS4_9allocatorIcEEEERKNS4_10shared_ptrINS_5SceneEEE+2664)
12-14 15:41:59.490   976   976 F DEBUG   :     #07 pc 000c3e43  /data/app/com.mapzen.tangram.android-1/lib/arm/libtangram.so (_ZN7Tangram11SceneLoader11applyConfigERKNSt3__110shared_ptrINS_5SceneEEE+2962)
12-14 15:41:59.490   976   976 F DEBUG   :     #08 pc 000b8809  /data/app/com.mapzen.tangram.android-1/lib/arm/libtangram.so (_ZNSt3__110__function6__funcIZN7Tangram3Map17applySceneUpdatesEvE3$_1NS_9allocatorIS4_EEFvvEEclEv+24)
12-14 15:41:59.490   976   976 F DEBUG   :     #09 pc 000bab1b  /data/app/com.mapzen.tangram.android-1/lib/arm/libtangram.so (_ZN7Tangram11AsyncWorker3runEv+318)
12-14 15:41:59.490   976   976 F DEBUG   :     #10 pc 000babed  /data/app/com.mapzen.tangram.android-1/lib/arm/libtangram.so (_ZNSt3__114__thread_proxyINS_5tupleIJMN7Tangram11AsyncWorkerEFvvEPS3_EEEEEPvS8_+76)
12-14 15:41:59.490   976   976 F DEBUG   :     #11 pc 0003fa3b  /system/lib/libc.so (_ZL15__pthread_startPv+30)
12-14 15:41:59.490   976   976 F DEBUG   :     #12 pc 0001a085  /system/lib/libc.so (__start_thread+6)
12-14 15:41:59.700  1391  6882 D N       : limitCPUFreq:: freq = 1632000

tallytalwar added a commit that referenced this issue Dec 14, 2016
- Cause was a bad JNI environment on startUrlRequest
- Fixed with attaching current url thread to the jni and grabbing jniEnvironment from it. (Apparently we already had a
wrapper class doing this, which was not used here)

Fixes #978
matteblair pushed a commit that referenced this issue Dec 14, 2016
- Cause was a bad JNI environment on startUrlRequest
- Fixed with attaching current url thread to the jni and grabbing jniEnvironment from it. (Apparently we already had a
wrapper class doing this, which was not used here)

Fixes #978
hjanetzek pushed a commit that referenced this issue Jan 12, 2017
rebased+squashed mbtiles branch from Nicholas Hallahan

Added SQLiteCpp

Added mbtiles parameter to data sources and scene.yam #960

Created MBTilesTileTask. #960

Basic MBTiles functionality working! #960

No need to make 2 vectors. Writing SQLite blob directly into rawTileData #960

Database and Statements are class members.

Now caching network tiles in MBTiles. #960

Now creating fresh MBTiles caches. Using schema from node-mbtiles. #960

Working on dynamically setting MBTiles from Tangram::Map interface. #960

Remove setMBTiles functions. Going with Scene Updates. Also isOfflineOnly is now hasNoURL. #960

MBTiles Scene Update working on Android when commenting out fonts. #978 #960

Create Map::setMBTiles and exposing it through JNI #960

Added MD5 hash for join key on MBTiles tile and images tables #960

Write to metadata table when setting up MBTiles #960

ClientGeoJsonSource will never have MBTiles #960

fixup

disable failing cpplint.py checks

Implemented mimeType method in TestDataSource

compression: identity when there is no compression

linux target_link_libraries includes dl in core

Cleaner way of setting target_link_libraries to include dl for Linux.
hjanetzek pushed a commit that referenced this issue Jan 20, 2017
rebased+squashed mbtiles branch from Nicholas Hallahan

Added SQLiteCpp

Added mbtiles parameter to data sources and scene.yam #960

Created MBTilesTileTask. #960

Basic MBTiles functionality working! #960

No need to make 2 vectors. Writing SQLite blob directly into rawTileData #960

Database and Statements are class members.

Now caching network tiles in MBTiles. #960

Now creating fresh MBTiles caches. Using schema from node-mbtiles. #960

Working on dynamically setting MBTiles from Tangram::Map interface. #960

Remove setMBTiles functions. Going with Scene Updates. Also isOfflineOnly is now hasNoURL. #960

MBTiles Scene Update working on Android when commenting out fonts. #978 #960

Create Map::setMBTiles and exposing it through JNI #960

Added MD5 hash for join key on MBTiles tile and images tables #960

Write to metadata table when setting up MBTiles #960

ClientGeoJsonSource will never have MBTiles #960

fixup

disable failing cpplint.py checks

Implemented mimeType method in TestDataSource

compression: identity when there is no compression

linux target_link_libraries includes dl in core

Cleaner way of setting target_link_libraries to include dl for Linux.
matteblair pushed a commit that referenced this issue Feb 8, 2017
rebased+squashed mbtiles branch from Nicholas Hallahan

Added SQLiteCpp

Added mbtiles parameter to data sources and scene.yam #960

Created MBTilesTileTask. #960

Basic MBTiles functionality working! #960

No need to make 2 vectors. Writing SQLite blob directly into rawTileData #960

Database and Statements are class members.

Now caching network tiles in MBTiles. #960

Now creating fresh MBTiles caches. Using schema from node-mbtiles. #960

Working on dynamically setting MBTiles from Tangram::Map interface. #960

Remove setMBTiles functions. Going with Scene Updates. Also isOfflineOnly is now hasNoURL. #960

MBTiles Scene Update working on Android when commenting out fonts. #978 #960

Create Map::setMBTiles and exposing it through JNI #960

Added MD5 hash for join key on MBTiles tile and images tables #960

Write to metadata table when setting up MBTiles #960

ClientGeoJsonSource will never have MBTiles #960

fixup

disable failing cpplint.py checks

Implemented mimeType method in TestDataSource

compression: identity when there is no compression

linux target_link_libraries includes dl in core

Cleaner way of setting target_link_libraries to include dl for Linux.
matteblair pushed a commit that referenced this issue Feb 8, 2017
rebased+squashed mbtiles branch from Nicholas Hallahan

Added SQLiteCpp

Added mbtiles parameter to data sources and scene.yam #960

Created MBTilesTileTask. #960

Basic MBTiles functionality working! #960

No need to make 2 vectors. Writing SQLite blob directly into rawTileData #960

Database and Statements are class members.

Now caching network tiles in MBTiles. #960

Now creating fresh MBTiles caches. Using schema from node-mbtiles. #960

Working on dynamically setting MBTiles from Tangram::Map interface. #960

Remove setMBTiles functions. Going with Scene Updates. Also isOfflineOnly is now hasNoURL. #960

MBTiles Scene Update working on Android when commenting out fonts. #978 #960

Create Map::setMBTiles and exposing it through JNI #960

Added MD5 hash for join key on MBTiles tile and images tables #960

Write to metadata table when setting up MBTiles #960

ClientGeoJsonSource will never have MBTiles #960

fixup

disable failing cpplint.py checks

Implemented mimeType method in TestDataSource

compression: identity when there is no compression

linux target_link_libraries includes dl in core

Cleaner way of setting target_link_libraries to include dl for Linux.
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

4 participants