Automating heap dumps from an Android device or emulator

Question

I've automated the process of extracting and converting a heap dump from an Android device/emulator.

I'd appreciate any feedback and improvements regarding style, bad practices, non-idiomatic code, performance or other issues you may find.

Also, dump_heap() function tries to wait for a daemon to complete and I'm wondering if there are better ways to handle the situation.

#!/bin/bash
#
# Extracts a heap dump from a device/emulator. 
# Assumes the appropriate tools (adb, hprof-conv) are included in the path.
SERIAL=""
PACKAGE=""
FILE=""
usage() {
 echo "Usage: -s <serialNumber> -p <package> -f <file>"
 exit 1
}
err() {
 local msg="1ドル"
 echo "$msg" >&2
 exit 1
}
create_file() {
 local file=1ドル
 local dir=$(dirname "$file")
 mkdir -p -- "$dir" && touch -- "$file" 
 if [ ! -e "$file" ]; then 
 err "File $file could not be created."
 fi
}
file_size_on_device() {
 local adbcmd="adb -s $SERIAL"
 local file="1ドル"
 local adbinfo="$adbcmd shell ls -l $file"
 $adbinfo | awk '{print 4ドル}'
}
dump_heap() {
 local adbcmd="adb -s $SERIAL"
 local pid="1ドル"
 local file="2ドル"
 local dir=$(dirname "$file")
 # Remove any existing tmp files we created. Ideally, there should be none.
 local exists=$($adbcmd shell ls "$dir" | grep "$file")
 if [ -n "$exists" ]; then
 $adbcmd shell rm "$file"
 fi
 echo -n "Dumping heap "
 # Dump the heap to a tmp file on a device/emulator
 $adbcmd shell am dumpheap "$pid" "$file"
 # Beacuse the previous cmd runs as a daemon, we have to wait for it to 
 # finish. The following checks the tmp file size continuously, stopping
 # when it's not changing anymore. 
 sleepinterval=0.5
 s0=-1
 sleep $sleepinterval
 s1=$(file_size_on_device "$file")
 while ((s1 > s0))
 do 
 echo -n "."
 sleep $sleepinterval
 let s0=$s1
 let s1=$(file_size_on_device "$file")
 done
 echo " Done"
}
extract_from_device() { 
 local adbcmd="adb -s $SERIAL"
 local pid=$($adbcmd shell ps | grep $PACKAGE | awk '{print 2ドル}')
 if [ -z "${pid}" ]; then
 err "PID for $PACKAGE not found. Is your app installed and running?"
 fi
 # Dump heap to tmp file on a device
 local file="/sdcard/hprof___tmp"
 dump_heap "$pid" "$file"
 # Extract
 local tmp="$FILE-nonconv" 
 create_file $tmp
 $adbcmd pull -p $file $tmp
 # Convert
 if [ -s "$tmp" ]; then
 create_file $FILE 
 hprof-conv $tmp $FILE
 else
 echo "No data to convert."
 fi
 # Finish
 $adbcmd shell rm "$file"
 rm -- $tmp
 echo "Done, converted heap extracted to $FILE"
}
while getopts ":s::p::f:" opt; do
 case $opt in
 s) SERIAL="$OPTARG";;
 p) PACKAGE="$OPTARG";;
 f) FILE="$OPTARG";;
 *) usage;;
 esac
done
if [ -z "${SERIAL}" ] || [ -z "${PACKAGE}" ] || [ -z "${FILE}" ]; then
 usage
else 
 version=$(adb -s "$SERIAL" shell getprop ro.build.version.sdk | tr -d '\r')
 if [ "$version" -gt "10" ]; then
 extract_from_device
 else 
 err "The target device/emulator must be API 11 or above."
 fi
fi

The above is part of Dumpey, which helps you

• run the monkey on multiple devices or emulators and
• make converted memory heap dumps before and after it

• Etan Reisner: I believe that getopts string should just be :s:p:f: for silent mode and a required argument for each of the s, p and f flags.
• Etan Reisner: Storing commands in strings is generally not a good idea (as with adbcmd and adbinfo). Anything complicated breaks. If you must store it using an array is much better but it doesn't look like you need to here.
• Tadej: Thanks, I’ll fix what you’ve pointed out. If there’s anything else fishy, please let me know.

Answer 1

First of all, your code passes on http://www.shellcheck.net/# with flying colors, congrats!

Simplify variable initializations

Here you can drop all the "", it will be the same thing but easier to write:

SERIAL=""
PACKAGE=""
FILE=""

That is, like this:

SERIAL=
PACKAGE=
FILE=

Check if a regular file exists with -f instead of -e

I admire your paranoia, with all the --:

mkdir -p -- "$dir" && touch -- "$file" 
if [ ! -e "$file" ]; then 
 err "File $file could not be created."
fi

But I would also recommend to use -f instead of -e, as you are really expecting a regular file, and -e is not necessary a regular file, can be a directory too.

Avoid putting commands into variables, especially when used only once

About this code:

local adbinfo="$adbcmd shell ls -l $file"
$adbinfo | awk '{print 4ドル}'

When the command in $adbinfo becomes anything non-trivial, running the command with $adbinfo | ... may not work as expected. As long as you keep that in mind, this is fine. However, since you only use this command once in the file_size_on_device function, I recommend to not use $adbinfo at all, but the command directly:

$adbcmd shell ls -l $file | awk '{print 4ドル}'

Even so, just keep in mind that the same dangers exist for $adbcmd as for $adbinfo earlier. But at least we removed one layer of danger ($adbinfo), so that's a good thing.

Simplify ls | grep, use just ls when it's good enough

In dump_heap, I'm wondering if this actually works as expected:

# Remove any existing tmp files we created. Ideally, there should be none.
local exists=$($adbcmd shell ls "$dir" | grep "$file")
if [ -n "$exists" ]; then
 $adbcmd shell rm "$file"
fi

In particular, it seems to me the rm command should be like this:

$adbcmd shell rm "$dir/$file"

Because ls "$dir" | grep "$file" suggests that $file is just a filename, not an absolute path, inside the directory $dir.

Also, the call to grep seems unnecessary, a single ls command should do:

local exists=$($adbcmd shell ls "$dir/$file")

Simplifying conditions

Still in dump_heap, when checking if $exists is non-empty, instead of -n "$exists", you can just drop the -n, simply [ "$exists" ] is the same thing.

Lastly, I'm wondering if the whole thing can be simplified to:

# Remove any existing tmp files we created. Ideally, there should be none.
$adbcmd shell ls "$dir/$file" &>/dev/null && $adbcmd shell rm "$dir/$file"

Prefer printf over echo for non-trivial printing

The various flags of echo are unreliable. Depending on your system, they might work as expected or not. Rather than using any of the flags of echo, I prefer printf. So instead of:

echo -n "Dumping heap "

I recommend:

printf "Dumping heap "

However, I prefer echo when printing trivial text, without using any flags, and when I want a terminating newline.

Prefer fewer processes

In this pipeline there are 3 processes: adb, grep and awk

local pid=$($adbcmd shell ps | grep $PACKAGE | awk '{print 2ドル}')

Especially in cases when an awk follows a grep, one begs the question if the task of grep can be included in the awk command. And in this case yes:

local pid=$($adbcmd shell ps | awk -v pkg=$PACKAGE '0ドル ~ pkg {print 2ドル}')

However, a little laziness is not against the spirit of good shell scripting. Your version with grep is shorter and simpler, and on most days I would go with it too. So this remark is just for the record, and for the meticulous.

Simplify condition

Instead of this:

if [ -z "${SERIAL}" ] || [ -z "${PACKAGE}" ] || [ -z "${FILE}" ]; then

I suggest to simplify like this:

if ! [ "$SERIAL" -a "$PACKAGE" -a "$FILE" ]; then

• Tadej: Wow, thanks for a great review janos, this is exactly what I was looking for!