Merge pull request #85 from nicolasbrailo/leak-hunting-with-StrictMode

Holiday Hacking 3: Leak hunting with strict mode

app/src/main/java/com/nicobrailo/pianoli/MainActivity.java

@@ -5,6 +5,7 @@
 import android.media.MediaPlayer;
 import android.os.Build;
 import android.os.Bundle;
+import android.util.Log;
 import android.view.View;
 import android.view.Window;
 import android.view.WindowManager;
@@ -72,6 +73,7 @@ protected void onResume() {
 
     @Override
     public void requestConfig() {
+        Log.i("PianOli::main", "opening config");
         // play a loud, distinct noise to alert any adults nearby that *someone*
         // (potentially a half-supervised child) has managed to break out of the app.
         final MediaPlayer snd = MediaPlayer.create(this, R.raw.alert);
@@ -92,7 +94,7 @@ public void requestConfig() {
 
     @Override
     public void showConfigTooltip() {
-        Toast toast = Toast.makeText(getApplicationContext(), R.string.config_tooltip, Toast.LENGTH_LONG);
+        Toast toast = Toast.makeText(this, R.string.config_tooltip, Toast.LENGTH_LONG);
         toast.show();
     }
 }

app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java

@@ -52,6 +52,7 @@ class PianoCanvas extends SurfaceView implements SurfaceHolder.Callback, PianoLi
     private final Theme theme;
 
     private Map<Integer, Integer> touch_pointer_to_keys = new HashMap<>();
+    private SoundSet soundSet;
 
     public PianoCanvas(Context context, AttributeSet as) {
         this(context, as, 0);
@@ -71,18 +72,18 @@ public PianoCanvas(Context context, AttributeSet as, int defStyle) {
             Display display = ctx.getWindowManager().getDefaultDisplay();
             display.getSize(screen_size);
         } catch (ClassCastException ex) {
-            Log.e("PianOli::DrawingCanvas", "Can't read screen size");
+            Log.e("PianOli::PianoCanvas", "Can't read screen size");
             throw ex;
         }
 
         screen_size_x = screen_size.x;
         screen_size_y = screen_size.y;
-        final String soundset = Preferences.selectedSoundSet(context);
+        final String prefSoundset = Preferences.selectedSoundSet(context);
 
         // DANGER ZONE: !! delicate ordering dependencies in this block !!
         appConfigTrigger = new AppConfigTrigger();
         // gotcha: gets just-created appConfigHandler via field
-        reInitPiano(context, soundset); // danger: impl leaks `this` pointer before ctor finished
+        reInitPiano(context, prefSoundset); // danger: impl leaks `this` pointer before ctor finished
         // gotcha: needs piano field that was just initialised by reInitPiano above
         this.bevelWidth = piano.get_keys_width() * BEVEL_RATIO;
 
@@ -96,7 +97,8 @@ public PianoCanvas(Context context, AttributeSet as, int defStyle) {
                 ", there are " + piano.get_keys_count() + " keys");
     }
 
-    public void reInitPiano(Context context, String soundset) {
+    public void reInitPiano(Context context, String prefSoundset) {
+        Log.i("PianOli::PianoCanvas", "re-initialising Piano");
         this.piano = new Piano(screen_size_x, screen_size_y);
 
         // for config trigger updates
@@ -105,9 +107,12 @@ public void reInitPiano(Context context, String soundset) {
         // to redraw on key-touches, must be after config handler to ensure its input is also drawn
         piano.addListener(this);
 
+        if (soundSet != null) {
+            soundSet.close();
+        }
         // Respond musically to key-presses: listen with a "soundMaker"
         // Use "strategy pattern" to deal with the two possible key-to-note mappings:
-        SoundSet soundSet = new SampledSoundSet(context, soundset);
+        soundSet = new SampledSoundSet(context, prefSoundset);
         PianoListener soundMaker;
         if (Preferences.areMelodiesEnabled(context)) {
             // "melodic" strategy: next note is determined by melody
@@ -119,6 +124,7 @@ public void reInitPiano(Context context, String soundset) {
             soundMaker = new StraightKeySoundMaker(soundSet);
         }
         piano.addListener(soundMaker);
+        Log.i("PianOli::PianoCanvas", "re-initialising Piano - DONE");
     }
 
     public void setConfigRequestCallback(AppConfigTrigger.AppConfigCallback cb) {
@@ -259,6 +265,7 @@ void draw_icon_on_black_key(final Canvas canvas, final Drawable icon, int key_id
 
     @Override
     public void surfaceCreated(@NonNull SurfaceHolder surfaceHolder) {
+        Log.i("PianOli::PianoCanvas", "surfaceCreated");
         redraw(surfaceHolder);
     }
 
@@ -382,8 +389,12 @@ public boolean onTouchEvent(MotionEvent event) {
 
 
     @Override
-    public void surfaceChanged(@NonNull SurfaceHolder surfaceHolder, int i, int i1, int i2) {}
+    public void surfaceChanged(@NonNull SurfaceHolder surfaceHolder, int i, int i1, int i2) {
+        Log.i("PianOli::PianoCanvas", "surfaceChanged: ignoring!");
+    }
 
     @Override
-    public void surfaceDestroyed(@NonNull SurfaceHolder surfaceHolder) {}
+    public void surfaceDestroyed(@NonNull SurfaceHolder surfaceHolder) {
+        Log.i("PianOli::PianoCanvas", "surfaceDestroyed: ignoring!");
+    }
 }

app/src/main/java/com/nicobrailo/pianoli/SettingsFragment.java

@@ -42,7 +42,7 @@ public void onAttach (@NonNull Context context) {
             Toast toast = Toast.makeText(context, msg, Toast.LENGTH_LONG);
             toast.show();
 
-            Log.d("PianOli::Activity", "Sound assets not available: piano will have no sound!");
+            Log.e("PianOli::Settings", "Sound assets not available: piano will have no sound!");
         }
     }
 

app/src/main/java/com/nicobrailo/pianoli/sound/SampledSoundSet.java

@@ -1,6 +1,7 @@
 package com.nicobrailo.pianoli.sound;
 
 import android.content.Context;
+import android.content.res.AssetFileDescriptor;
 import android.content.res.AssetManager;
 import android.media.SoundPool;
 import android.util.Log;
@@ -18,7 +19,7 @@
  * object, and replace it with a new instance.
  * </p>
  */
-public class SampledSoundSet implements AutoCloseable, SoundSet {
+public class SampledSoundSet implements SoundSet {
     /**
      * The amount of samples/notes in each sound-set.
      *
@@ -38,8 +39,11 @@ public class SampledSoundSet implements AutoCloseable, SoundSet {
      * Resource handles to the mp3 samples of our currently active soundset, one for each note
      */
     private final int[] samples;
+    private final String name;
 
     public SampledSoundSet(final Context context, String soundSetName) {
+        this.name = soundSetName;
+
         pool = new SoundPool.Builder()
                 .setMaxStreams(7)   // Play max N concurrent sounds
                 .build();
@@ -90,11 +94,14 @@ public SampledSoundSet(final Context context, String soundSetName) {
     private static int loadNoteFd(AssetManager am, SoundPool pool, String soundSetName, int noteNum) throws IOException {
         String assetFolder = "sounds/" + SoundSet.addPrefix(soundSetName) + "/";
         String fileName = String.format(Locale.ROOT, "n%02d.mp3", noteNum); // root locale OK for number-formatting.
-        return pool.load(am.openFd(assetFolder + fileName), 1);
+        try (AssetFileDescriptor afd = am.openFd(assetFolder + fileName)) {
+            return pool.load(afd, 1);
+        }
     }
 
     @Override
     public void close() {
+        Log.d("PianOli::SoundSet", "releasing soundset " + name);
         pool.release();
     }
 

app/src/main/java/com/nicobrailo/pianoli/sound/SoundSet.java

@@ -9,10 +9,8 @@
 
 /**
  * {@link SoundSet}s represent PianOli's capability to make musical noise in response to keypresses.
- *
- * This interface
  */
-public interface SoundSet {
+public interface SoundSet extends AutoCloseable {
     /**
      * prefix for soundset stuff, both of asset-folders containing instrument samples and translation string keys.
      *
@@ -74,4 +72,6 @@ static List<String> getAvailableSoundsets(AssetManager am) {
     }
 
     void playNote(int keyIdx);
+
+    void close();
 }

app/src/test/java/com/nicobrailo/pianoli/sound/SpySoundSet.java

@@ -1,7 +1,5 @@
 package com.nicobrailo.pianoli.sound;
 
-import com.nicobrailo.pianoli.sound.SoundSet;
-
 /**
  * observable dummy {@link SoundSet}, for test purposes.
  */
@@ -13,4 +11,9 @@ public class SpySoundSet implements SoundSet {
     public void playNote(int keyIdx) {
         lastPlayed = keyIdx;
     }
+
+    @Override
+    public void close() {
+        // dummy implementation, do nothing
+    }
 }