Android, RWOPS strikes again

Gabriel_Jacobo · July 20, 2012, 4:31pm

Hi everyone, I think there’s something fundamentally wrong in the RWOps
backend for Android. There’s been quite a few fixes for it in the last few
months (threading problems, file seeking problems, etc), but I’m afraid
when combined with SDL_ttf something inside Android just gives up…

To anyone interested in verifying this, I’m testing with this APK I built (
http://mdqinc.com/com.mdqinc.bunnymarkx.apk), it’s a “Bunnymark” sort of
app, it shows bunnies jumping around to see how many it can display…when
it works.

The problem started when I added an on screen counter which shows the
number of bunnies, using SDL_ttf. When I try to load the font, I get a
deadlock. It’s not always in the same spot, and, to make this a bonafide
hardcore bug, a very few times (one out of 20 runs maybe), it doesn’t
happen at all…but at least in my ICS loaded Asus Transformer TF101 it
happens fairly consistently.

Most of the locks happen when calling:

jobject byteBuffer = mEnv->NewDirectByteBuffer(buffer, bytesRemaining);

or

int result = mEnv->CallIntMethod(readableByteChannel, readMethod,
byteBuffer);

Which clearly points out (at least to me) to a problem inside the Java VM.
I’ve added a “FileSeek counter” in the APK that counts the number of times
Android_JNI_FileSeek is called, most of the deadlocks happen when the
counter is between 99 and 105, a fairly consistent number as well.

Strangely enough, I’ve never had this issue with SDL_image, I believe this
is due to the way Freetype operates, going forward and backward on the file
several times.

If this is a fundamental issue with the Android JNI stuff, I don’t think
there’s other way around it that loading the font file entirely in memory
and pointing RWOps to that buffer…and that certainly leaves open the
question of what other library can make the Android system deadlock like
this.

If anyone can test the APK and send me feedback about it (you got to
monitor the adb logcat | grep SDL output when it’s running) or any idea on
how to make a proper fix for this problem, I’ll appreciate it.–
Gabriel.

Eric_Wing · July 20, 2012, 6:45pm

So I haven’t kept up with this problem, but is SDL using (or
experimented with) AAssetManager which is the public NDK API (Android
2.3) for accessing files? I kind of hope/expect that this API will
deal with the thread safety issues correctly and be much easier/less
error prone to use than crossing the JNI boundary manually.

-EricOn 7/20/12, Gabriel Jacobo wrote:

Hi everyone, I think there’s something fundamentally wrong in the RWOps
backend for Android. There’s been quite a few fixes for it in the last few
months (threading problems, file seeking problems, etc), but I’m afraid
when combined with SDL_ttf something inside Android just gives up…

To anyone interested in verifying this, I’m testing with this APK I built (
http://mdqinc.com/com.mdqinc.bunnymarkx.apk), it’s a “Bunnymark” sort of
app, it shows bunnies jumping around to see how many it can display…when
it works.

The problem started when I added an on screen counter which shows the
number of bunnies, using SDL_ttf. When I try to load the font, I get a
deadlock. It’s not always in the same spot, and, to make this a bonafide
hardcore bug, a very few times (one out of 20 runs maybe), it doesn’t
happen at all…but at least in my ICS loaded Asus Transformer TF101 it
happens fairly consistently.

Most of the locks happen when calling:

jobject byteBuffer = mEnv->NewDirectByteBuffer(buffer, bytesRemaining);

or

int result = mEnv->CallIntMethod(readableByteChannel, readMethod,
byteBuffer);

Which clearly points out (at least to me) to a problem inside the Java VM.
I’ve added a “FileSeek counter” in the APK that counts the number of times
Android_JNI_FileSeek is called, most of the deadlocks happen when the
counter is between 99 and 105, a fairly consistent number as well.

Strangely enough, I’ve never had this issue with SDL_image, I believe this
is due to the way Freetype operates, going forward and backward on the file
several times.

If this is a fundamental issue with the Android JNI stuff, I don’t think
there’s other way around it that loading the font file entirely in memory
and pointing RWOps to that buffer…and that certainly leaves open the
question of what other library can make the Android system deadlock like
this.

If anyone can test the APK and send me feedback about it (you got to
monitor the adb logcat | grep SDL output when it’s running) or any idea on
how to make a proper fix for this problem, I’ll appreciate it.

–
Gabriel.

–
Beginning iPhone Games Development
http://playcontrol.net/iphonegamebook/

Gabriel_Jacobo · July 20, 2012, 6:52pm

2012/7/20 Eric Wing > On 7/20/12, Gabriel Jacobo <@Gabriel_Jacobo> wrote:

So I haven’t kept up with this problem, but is SDL using (or
experimented with) AAssetManager which is the public NDK API (Android
2.3) for accessing files? I kind of hope/expect that this API will
deal with the thread safety issues correctly and be much easier/less
error prone to use than crossing the JNI boundary manually.

-Eric

I haven’t tried it, but it looks like it may be the way out of this…until
we hit the next bug at least!

–
Gabriel.

Tim_Angus · July 20, 2012, 7:06pm

I had deliberately avoided using this as it would raise the minimum API level. For sure, there are much better options for doing the FS backend (and other things) if you’re willing to push it up. The NativeActivity stuff in particular would make things a lot easier to deal with.

To be honest I’m not bothered on what the minimum API level is, within reason. It’s not a decision I would like to make though :).On Fri, 20 Jul 2012, 19:52:42 BST, Gabriel Jacobo wrote:

2012/7/20 Eric Wing

On 7/20/12, Gabriel Jacobo wrote:

So I haven’t kept up with this problem, but is SDL using (or
experimented with) AAssetManager which is the public NDK API (Android
2.3) for accessing files? I kind of hope/expect that this API will
deal with the thread safety issues correctly and be much easier/less
error prone to use than crossing the JNI boundary manually.

-Eric

I haven’t tried it, but it looks like it may be the way out of
this…until we hit the next bug at least!

–
Gabriel.

Andre_Heider · July 20, 2012, 7:42pm

Hi,

by what I see in Android_JNI_FileOpen I can maybe point to an nicer
and better performing implementation, which might not be affected by
this JNI locking problem (unsure if I understand the problem
correctly, I haven’t used SDL on andoid).

We use this since day one for ScummVM, and it works on a metric ton of
devices on API level 6:

github.com

scummvm/scummvm/blob/master/backends/platform/android/asset-archive.cpp#L473


/* ScummVM - Graphic Adventure Engine
*
* ScummVM is the legal property of its developers, whose names
* are too numerous to list here. Please refer to the COPYRIGHT
* file distributed with this source distribution.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*
*/


#if defined(__ANDROID__)


#include <sys/types.h>
#include <unistd.h>


#include "common/str.h"
#include "common/stream.h"
#include "common/util.h"
#include "common/archive.h"
#include "common/debug.h"
#include "common/textconsole.h"


#include "backends/platform/android/jni.h"
#include "backends/platform/android/asset-archive.h"


#include <android/asset_manager.h>
#include <android/asset_manager_jni.h>


class AssetInputStream : public Common::SeekableReadStream {
public:
	AssetInputStream(AAssetManager *as, const Common::String &path);
	virtual ~AssetInputStream();


	virtual bool eos() const { return _eos; }


	virtual void clearErr() {_eos = false; }


	virtual uint32 read(void *dataPtr, uint32 dataSize);


	virtual int32 pos() const { return _pos; }


	virtual int32 size() const { return _len; }


	virtual bool seek(int32 offset, int whence = SEEK_SET);


private:
	void close();
	AAsset *_asset;


	uint32 _pos;
	uint32 _len;
	bool _eos;
};


AssetInputStream::AssetInputStream(AAssetManager *as, const Common::String &path) :
	_eos(false), _pos(0) {
	_asset = AAssetManager_open(as, path.c_str(), AASSET_MODE_RANDOM);
	_len = AAsset_getLength(_asset);
}


AssetInputStream::~AssetInputStream() {
}


void AssetInputStream::close() {
	AAsset_close(_asset);
}


uint32 AssetInputStream::read(void *dataPtr, uint32 dataSize) {
	uint32 readlen = AAsset_read(_asset, dataPtr, dataSize);
	_pos += readlen;
	if (readlen != dataSize) {
		_eos = true;
	}
	return readlen;
}


bool AssetInputStream::seek(int32 offset, int whence) {
	int res = AAsset_seek(_asset, offset, whence);
	if (res == -1) {
		return false;
	}
	if (whence == SEEK_CUR) {
		_pos += offset;
	} else if (whence == SEEK_SET) {
		_pos = offset;
	} else if (whence == SEEK_END) {
		_pos = _len + offset;
	}
	assert(_pos <= _len);
	_eos = false;
	return true;
}


AndroidAssetArchive::AndroidAssetArchive(jobject am) : _hasCached(false) {
	JNIEnv *env = JNI::getEnv();


	_am = AAssetManager_fromJava(env, am);
}


AndroidAssetArchive::~AndroidAssetArchive() {
}


bool AndroidAssetArchive::hasFile(const Common::String &name) const {
	AAsset *asset = AAssetManager_open(_am, name.c_str(), AASSET_MODE_RANDOM);
	bool exists = false;
	if (asset != NULL) {
		exists = true;
		AAsset_close(asset);
	}
	return exists;
}


int AndroidAssetArchive::listMembers(Common::ArchiveMemberList &member_list) const {
	if (_hasCached) {
		member_list.insert(member_list.end(), _cachedMembers.begin(), _cachedMembers.end());
		return _cachedMembers.size();
	}


	int count = 0;
	AAssetDir *dir = AAssetManager_openDir(_am, "");
	const char *file = AAssetDir_getNextFileName(dir);


	while (file) {
		member_list.push_back(getMember(file));
		++count;
		file = AAssetDir_getNextFileName(dir);
	}
	AAssetDir_close(dir);


	_cachedMembers = Common::ArchiveMemberList(member_list);
	_hasCached = true;


	return count;
}


const Common::ArchiveMemberPtr AndroidAssetArchive::getMember(const Common::String &name) const {
	return Common::ArchiveMemberPtr(new Common::GenericArchiveMember(name, this));
}


Common::SeekableReadStream *AndroidAssetArchive::createReadStreamForMember(const Common::String &path) const {
	if (!hasFile(path)) {
		return nullptr;
	}
	return new AssetInputStream(_am, path);
}


#endif

github.com

scummvm/scummvm/blob/master/backends/platform/android/asset-archive.cpp#L239


/* ScummVM - Graphic Adventure Engine
*
* ScummVM is the legal property of its developers, whose names
* are too numerous to list here. Please refer to the COPYRIGHT
* file distributed with this source distribution.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*
*/


#if defined(__ANDROID__)


#include <sys/types.h>
#include <unistd.h>


#include "common/str.h"
#include "common/stream.h"
#include "common/util.h"
#include "common/archive.h"
#include "common/debug.h"
#include "common/textconsole.h"


#include "backends/platform/android/jni.h"
#include "backends/platform/android/asset-archive.h"


#include <android/asset_manager.h>
#include <android/asset_manager_jni.h>


class AssetInputStream : public Common::SeekableReadStream {
public:
	AssetInputStream(AAssetManager *as, const Common::String &path);
	virtual ~AssetInputStream();


	virtual bool eos() const { return _eos; }


	virtual void clearErr() {_eos = false; }


	virtual uint32 read(void *dataPtr, uint32 dataSize);


	virtual int32 pos() const { return _pos; }


	virtual int32 size() const { return _len; }


	virtual bool seek(int32 offset, int whence = SEEK_SET);


private:
	void close();
	AAsset *_asset;


	uint32 _pos;
	uint32 _len;
	bool _eos;
};


AssetInputStream::AssetInputStream(AAssetManager *as, const Common::String &path) :
	_eos(false), _pos(0) {
	_asset = AAssetManager_open(as, path.c_str(), AASSET_MODE_RANDOM);
	_len = AAsset_getLength(_asset);
}


AssetInputStream::~AssetInputStream() {
}


void AssetInputStream::close() {
	AAsset_close(_asset);
}


uint32 AssetInputStream::read(void *dataPtr, uint32 dataSize) {
	uint32 readlen = AAsset_read(_asset, dataPtr, dataSize);
	_pos += readlen;
	if (readlen != dataSize) {
		_eos = true;
	}
	return readlen;
}


bool AssetInputStream::seek(int32 offset, int whence) {
	int res = AAsset_seek(_asset, offset, whence);
	if (res == -1) {
		return false;
	}
	if (whence == SEEK_CUR) {
		_pos += offset;
	} else if (whence == SEEK_SET) {
		_pos = offset;
	} else if (whence == SEEK_END) {
		_pos = _len + offset;
	}
	assert(_pos <= _len);
	_eos = false;
	return true;
}


AndroidAssetArchive::AndroidAssetArchive(jobject am) : _hasCached(false) {
	JNIEnv *env = JNI::getEnv();


	_am = AAssetManager_fromJava(env, am);
}


AndroidAssetArchive::~AndroidAssetArchive() {
}


bool AndroidAssetArchive::hasFile(const Common::String &name) const {
	AAsset *asset = AAssetManager_open(_am, name.c_str(), AASSET_MODE_RANDOM);
	bool exists = false;
	if (asset != NULL) {
		exists = true;
		AAsset_close(asset);
	}
	return exists;
}


int AndroidAssetArchive::listMembers(Common::ArchiveMemberList &member_list) const {
	if (_hasCached) {
		member_list.insert(member_list.end(), _cachedMembers.begin(), _cachedMembers.end());
		return _cachedMembers.size();
	}


	int count = 0;
	AAssetDir *dir = AAssetManager_openDir(_am, "");
	const char *file = AAssetDir_getNextFileName(dir);


	while (file) {
		member_list.push_back(getMember(file));
		++count;
		file = AAssetDir_getNextFileName(dir);
	}
	AAssetDir_close(dir);


	_cachedMembers = Common::ArchiveMemberList(member_list);
	_hasCached = true;


	return count;
}


const Common::ArchiveMemberPtr AndroidAssetArchive::getMember(const Common::String &name) const {
	return Common::ArchiveMemberPtr(new Common::GenericArchiveMember(name, this));
}


Common::SeekableReadStream *AndroidAssetArchive::createReadStreamForMember(const Common::String &path) const {
	if (!hasFile(path)) {
		return nullptr;
	}
	return new AssetInputStream(_am, path);
}


#endif

It basically is just:
AssetManager.openFd().getFileDescriptor()
and you get a fd for native unistd access.

Maybe it helps,
AndreOn Fri, Jul 20, 2012 at 9:06 PM, Tim Angus wrote:

I had deliberately avoided using this as it would raise the minimum API level. For sure, there are much better options for doing the FS backend (and other things) if you’re willing to push it up. The NativeActivity stuff in particular would make things a lot easier to deal with.

Tim_Angus · July 22, 2012, 6:08pm

Haha. Well if that definitely works it’s probably the best way to go.On Fri, 20 Jul 2012 21:42:45 +0200 Andre wrote:

https://github.com/scummvm/scummvm/blob/master/backends/platform/android/asset-archive.cpp#L473
https://github.com/scummvm/scummvm/blob/master/backends/platform/android/asset-archive.cpp#L239

It basically is just:
AssetManager.openFd().getFileDescriptor()
and you get a fd for native unistd access.