Android support

[steeve]

hey @arvidn,

Here's my patch for Android support, in the android branch: 10ad404

As you can see it's pretty simple.

Patch version: https://github.com/steeve/libtorrent/commit/10ad4044383f4c0af7a7e00326a2a4d943449db3.patch

However, I think there is a buffer overflow somewhere. If I let it use valloc, sooner or later I end up with memory corruption, and pretty much never (rarely, but did happen) if memalign is used.

[steeve]

I'm not submitting a Pull Request by the way, so you can merge it yourself from the Patch version if you want.

[steeve]

The issue is confirmed when using TORRENT_DEBUG and TORRENT_DEBUG_BUFFERS:

When stopping the session:

assertion failed. Please file a bugreport at http://code.google.com/p/libtorrent/issues
Please include the following information:

version: 0.16.12.0
$Rev: 9199 $
file: 'bt_peer_connection.cpp'
line: 200
function: virtual libtorrent::bt_peer_connection::~bt_peer_connection()
expression: m_ses.is_network_thread()

stack:

The stack is empty, it seems the program loops.

[steeve]

Relevant line: https://github.com/steeve/libtorrent/blob/libtorrent-0_16_12/src/bt_peer_connection.cpp#L200

        bt_peer_connection::~bt_peer_connection()
        {
>>              TORRENT_ASSERT(m_ses.is_network_thread());
        }

[arvidn]

Thanks for the patch! It looks good. This line you ifdefed out could
probably just be removed. I believe it's a copy-paste error:

index f614531..7c3921d 100644
--- a/include/libtorrent/sha1_hash.hpp
+++ b/include/libtorrent/sha1_hash.hpp
@@ -276,7 +276,9 @@ CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
TO, PROCUREMENT OF
};

typedef sha1_hash peer_id;

+#ifndef TORRENT_ANDROID
typedef sha1_hash sha1_hash;
+#endif

#if TORRENT_USE_IOSTREAM

Also, I'm pretty sure you'll run into issues with large files (>2GB or
at least >4GB). The kernel still supports large files, there are just
no libc mappings for them. I believe we could still do it by calling
the syscalls directly, with syscall(). Something like this:

syscall(__NR_pread64, fd, buffer, size, 0, boost::uint32_t(offset),
boost::uint32_t(offset >> 32));

On Tue, Nov 26, 2013 at 3:04 AM, Steeve Morin notifications@github.comwrote:

I'm not submitting a Pull Request by the way, so you can merge it yourself
from the Patch version if you want.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-29283716
.

Arvid Norberg

[steeve]

Yeah I should probably have done it from the 0.16.12 branch. I'll do that if you want.

[steeve]

Also, I'm getting abysmal performance if I download to the sdcard (/sdcard). It starts fine, but then after 20-30s, it just goes down to 0. I'm guessing this is a cache issue.

So I guess more patches are needed to announce Android support ;)

[arvidn]

Here's my guess:

1. your SD card is formatted as FAT32
2. FAT32 does not support sparse files
3. libtorrent will write to it as if it did, forcing the kernel/filesystem
   to allocate space up-front

to verify it, you could try to enabling sequential download mode.

If this turns out to be the case, there's a technique I've been thinking
about for a while to deal with this:

1. each file keeps a window starting at the beginning of the files to a
   piece where at least 30% of the pieces inside the window have been
   downloaded.
2. the window is never smaller than some threshold, say 20MB.
3. except that the window is never larger than the entire file
4. pieces are only allowed to be downloaded within these windows
5. As pieces completes inside the window, it has to expand to maintain the
   30% ratio

On Tue, Nov 26, 2013 at 3:00 PM, Steeve Morin notifications@github.comwrote:

Also, I'm getting abysmal performance if I download to the sdcard (/sdcard).
It starts fine, but then after 20-30s, it just goes down to 0. I'm guessing
this is a cache issue.

So I guess more patches are needed to announce Android support ;)

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-29343883
.

Arvid Norberg

[steeve]

Wow that is a spot on guess. Nice one.

Here is my mount, for reference:

root@android:/ # mount
rootfs / rootfs ro,relatime 0 0
tmpfs /dev tmpfs rw,nosuid,relatime,mode=755 0 0
devpts /dev/pts devpts rw,relatime,mode=600 0 0
proc /proc proc rw,relatime 0 0
sysfs /sys sysfs rw,relatime 0 0
none /acct cgroup rw,relatime,cpuacct 0 0
tmpfs /mnt/secure tmpfs rw,relatime,mode=700 0 0
tmpfs /mnt/asec tmpfs rw,relatime,mode=755,gid=1000 0 0
tmpfs /mnt/obb tmpfs rw,relatime,mode=755,gid=1000 0 0
none /dev/cpuctl cgroup rw,relatime,cpu 0 0
/dev/block/mtdblock8 /system ext4 ro,noatime,nodiratime,barrier=1,data=ordered,noauto_da_alloc 0 0
/dev/block/mtdblock6 /data ext4 rw,nosuid,nodev,noatime,nodiratime,barrier=1,data=ordered,noauto_da_alloc 0 0
/dev/block/mtdblock5 /cache ext4 rw,nosuid,nodev,noatime,nodiratime,barrier=1,data=ordered,noauto_da_alloc 0 0
/sys/kernel/debug /sys/kernel/debug debugfs rw,relatime 0 0
/dev/block/vold/31:9 /mnt/sdcard vfat rw,dirsync,nosuid,nodev,noexec,relatime,uid=1000,gid=1015,fmask=0002,dmask=0002,allow_utime=0020,codepage=cp437,iocharset=iso8859-1,shortname=mixed,utf8,errors=remount-ro 0 0
/dev/block/vold/31:9 /mnt/secure/asec vfat rw,dirsync,nosuid,nodev,noexec,relatime,uid=1000,gid=1015,fmask=0002,dmask=0002,allow_utime=0020,codepage=cp437,iocharset=iso8859-1,shortname=mixed,utf8,errors=remount-ro 0 0
tmpfs /mnt/sdcard/.android_secure tmpfs ro,relatime,size=0k,mode=000 0 0
root@android:/ #

[steeve]

Could using full allocation work in that case?

I've tried to switch full allocation, but after allocation, libtorrent pretty much stopped doing anything, so I'm not sure

[steeve]

Okay so after much much more checking, I think I found the cause of the memory corruption issues I had.

If I build libtorrent without pool allocators, everything works fine :)

So I guess it should be made somewhere to disable pool allocators on Android. Wether in the code or in autoconf :)

[arvidn]

an alternative would be to fix the bug. would you mind building with
TORRENT_DEBUG_BUFFERS defined for instance?

it only works if you also disable the pool allocators, but it will add
heavy-weight debugging instrumentation. What it will do
is to allocate two guard pages (on on each side of the allocation) and make
those pages non-readable and non-writable. If any logic in libtorrent
attempts to access those pages, you'll get a segmentation fault right away
and hopefully be able to see where that's happening. It's also printing a
stack trace into the first of those pages of where it was allocated. That
may provide more hints on what the bug is.

On Thu, Nov 28, 2013 at 5:50 PM, Steeve Morin notifications@github.comwrote:

Okay so after much much more checking, I think I found the cause of the
memory corruption issues I had.

If I build libtorrent without pool allocators, everything works fine :)

So I guess it should be made somewhere to disable pool allocators on
Android. Wether in the code or in autoconf :)

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-29492050
.

Arvid Norberg

[steeve]

I already did, and appart from the message shown at #2 (comment) nothing was really apparent...

Here's what I did for the last week trying to hunt down the bug:

• Built with TORRENT_DEBUG and TORRENT_DEBUG_BUFFERS, but nothing really happened (apart from the failed assertion).
• Tried to build with -fstack-protector-all in case it was a stack overflow, but no luck either.
• I've build and tried using valgrind too, but found out the hard way valgrind doesn't work with Go programs.
• Finally I tried to make sure all my calls to libtorrent were made from the main thread (in case that was a threading issue, related to the failed assertion earlier), no luck

Is there something else I can do?

[steeve]

Also, most people will want to download to the sdcard, which is indeed FAT32 on most (all?) Android systems.

So first of all, it does work, thankfully I'm using sequential mode, but it is there a way to limit the number of "outer" chunks? I'm asking because sometimes it stalls because the driver is zerofilling the missing space, so I was wondering if I could limit that "outer-reach" somehow.

[steeve]

Because as it is, it should be said to people not to download to the SD card, or any FAT32 device.

I also tried to create a EXT4 disk image on the FAT32 fs, but it seems loopback device support is at best spotty on Android :(

[arvidn]

Does the EXT4 image on top of FAT32 work? i.e. does it mitigate the
allocation issue?

if you're downloading sequentially, snubbed peers will be downloading
sequentially from the end though, so I guess that should probably be
configurable for this purpose.

This is controlled from peer_connection::picker_options() (a function that
the piece picker calls to have the peer tell it which flags should be set).

On Sat, Nov 30, 2013 at 7:38 AM, Steeve Morin notifications@github.comwrote:

Because as it is, it should be said to people not to download to the SD
card, or any FAT32 device.

I also tried to create a EXT4 disk image on the FAT32 fs, but it seems
loopback device support is at best spotty on Android :(

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-29554451
.

Arvid Norberg

[mandrazhoid]

Hi @arvidn,

Thanks for hint to use syscalls. This looks to be a solution for large files on android. I was able to call it successully in case of ftruncate, but have troubles with _NR_pread64 and _NR_pwrite64.

[steeve]

Fyi, I believe Android support was merged a few months ago already (in the svn, not here which is a mirror)

[arvidn]

@mandrazhoid do you have a patch with those system calls I can take a look at, to see if I can spot anything? I would be happy to merge a fix for this.
@steeve Did your patch deal with large files and the fact that android's libc doesn't provide syscall stubs for file functions with 64 bit offsets?

[steeve]

@arvidn apparently there is a lseek64 in bionic: https://android.googlesource.com/platform/bionic/+/a89864a/libc/bionic/lseek64.c

[mandrazhoid]

@steeve I don't think that lseek64 is the only 64 bit function which is necessary.
as I understand ftruncate, readv, writev in android are 32 bit.

@arvidn Sorry, trying to solve this I've put so many debug and comments as well as not using #ifdef while debugging , so it would be too dirty for patch.
I'm trying to get __NR_ftruncate64 working first (was too optimistic to think that is was working).

I'm calling

if (syscall(__NR_ftruncate64, m_fd, file_offset + size) < 0)

instead of

if (ftruncate(m_fd, file_offset + size) < 0)

I get EFBIG error.

[arvidn]

I would expect readv and writev to be the same entrypoints regardless of whether the file is large or not. In libtorrent_aio, I'm also using pread/pwrite though (and even preadv and pwritev if on linux).

@mandrazhoid My understanding is that the 64 bit version takes a 64 bit fd argument too. I'm not sure how what you do will be implemented through ellipsis. Essentially, on a little-endian system, you have to push 4 words (a word is 4 bytes). fd, 0, offset-low-word, offset-high-word.

you could try to either cast fd into a uint64_t or insert the zero-padding and split up the offset manually. Or maybe it's enough to just insert the zero padding with what you have right now.

[steeve]

Apparently, looking https://github.com/tatsuhiro-t/aria2/blob/master/src/android/arm-ftruncate64.S they use the syscall too

[steeve]

Most importantly: https://github.com/android/platform_bionic/blob/master/libc/arch-arm/syscalls/ftruncate64.S

[mandrazhoid]

@arvidn isn't fd argument always int, even for 64 bit functions?

[steeve]

@mandrazhoid it is:

int ftruncate64 (int fd, __off64_t length)

[arvidn]

Ok. I guess the zero should be considered padding then.
On Mar 11, 2014 4:08 PM, "Steeve Morin" notifications@github.com wrote:

@mandrazhoid https://github.com/mandrazhoid it is:

int ftruncate64 (int fd, __off64_t length)

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-37357908
.

[mandrazhoid]

still struggling with this syscall. got it partially working... works fine for small files, still doesn't work for > 2gb.

uint32_t low = length & 0xffffffff ;
uint32_t high = length >> 32 ;
return syscall(__NR_ftruncate64, file_descriptor , 0, low, high );

[arvidn]

that looks right to me. what's the symptom when it fails? I take it file_descriptor is an "int", right?

[mandrazhoid]

yes, file_descriptor is int.
i get EINVAL Invalid argument when calling it for large file. Torrent goes into endless checking cycle.