Android studio: thread app not returning same value as non thread app

Question

I'm working on an app that goes through all of the phone directories, and collecting all the songs. When i run it normally it works fine, just takes about 6 seconds to go through everything, and causes the app to skip a lot of frames. I changed it so every time a file is found, a different thread reads the metadata and saves it. I'm also waiting for all of them in the end, because after that I'm trying to use that list. All of a sudden several of the song are null, even though they're not initialized as such anywhere. What can cause that? an app that works fine, but not with threads..?

the constructor that calls the search:

phoneSongsList = new Playlist();
findSongs(Environment.getExternalStorageDirectory().getAbsolutePath()); //.concat("/Music")
for (Thread thread : threads) {
    try {
        thread.join();
    } catch (InterruptedException e) {
        e.printStackTrace();
    }
}

The function that recursively looks for songs:

public void findSongs(String path) {
    File home = new File(path);
    for (final File file : home.listFiles()) {
        if (file.isDirectory())
            findSongs(path.concat("/" + file.getName()));
        else if (isAcceptableExtension(file.getName())) {
            Thread t = new Thread(new Runnable() {
                @Override
                public void run() {
                    phoneSongsList.add(fileToSong(file));
                }
            });
            t.start();
            threads.add(t);
        }
    }
}

The function that converts the file to a song object:

private Song fileToSong(File file) {
    final Album album = new Album();
    Song song = new Song();
    song.setName(file.getName().substring(0, (file.getName().length() - 4))); // remove suffix
    song.setPath(file.getPath());

    final MediaMetadataRetriever metaRetriever = new MediaMetadataRetriever();
    metaRetriever.setDataSource(file.getPath());

    song.setArtists(metaRetriever.extractMetadata(MediaMetadataRetriever.METADATA_KEY_ARTIST));
    album.setName(metaRetriever.extractMetadata(MediaMetadataRetriever.METADATA_KEY_ALBUM));
    album.setYear(metaRetriever.extractMetadata(MediaMetadataRetriever.METADATA_KEY_YEAR));
    album.setCover(metaRetriever.getEmbeddedPicture(), context);
    song.setAlbum(album);
    song.setDuration(Long.parseLong(metaRetriever.extractMetadata(MediaMetadataRetriever.METADATA_KEY_DURATION)));
    song.setGenre(metaRetriever.extractMetadata(MediaMetadataRetriever.METADATA_KEY_GENRE));

    metaRetriever.release();
    return song;
}

The Playlist.add function:

public void add(Song song) {
    add(list.size(), song);
}
public void add(int index, Song song) {
    if(song==null)
        return;
    if (index > list.size())
        index = list.size();
    if (list.contains(song))
        list.remove(song);
    list.add(index, song);
}

Even when i explicitly specify that no null objects would be added to the list, it runs fine when it saves the songs, but give a null error when trying to read. Each time i run different songs and a different number of songs are set to null.

Please help.

Answer 1

You're dynamically trying to add new Threads to the list of threads on other threads, but reading that thread on one thread. That means you'll finish the loop on some subset of those threads before all of them are added. This entire approach is one big race condition.

This isn't something threads are going to speed up much, and you're doing your threading all wrong anyway. Throw this out and just do it on a single background thread, and do not join on that thread (or you may as well do it sequentially)- have it post back to the main thread when done.