From 72cccbde57cab5b48f93eb3f4d2288871b6eafbe Mon Sep 17 00:00:00 2001 From: Alexey Kulish Date: Wed, 14 Oct 2015 19:51:59 +0300 Subject: [PATCH] Downloads concurrency crash fix --- .../plus/download/BaseDownloadActivity.java | 36 ++++++-- .../plus/download/DownloadIndexesThread.java | 90 ++++++++++--------- .../plus/download/items/ItemsListBuilder.java | 27 +++--- .../download/items/RegionItemsFragment.java | 4 +- .../download/items/SearchItemsFragment.java | 7 +- .../download/items/VoiceDialogFragment.java | 3 +- .../download/items/VoiceItemsFragment.java | 2 +- .../download/items/WorldItemsFragment.java | 48 +++++----- 8 files changed, 118 insertions(+), 99 deletions(-) diff --git a/OsmAnd/src/net/osmand/plus/download/BaseDownloadActivity.java b/OsmAnd/src/net/osmand/plus/download/BaseDownloadActivity.java index 6780d4e9e2..72ddee3953 100644 --- a/OsmAnd/src/net/osmand/plus/download/BaseDownloadActivity.java +++ b/OsmAnd/src/net/osmand/plus/download/BaseDownloadActivity.java @@ -27,6 +27,7 @@ import java.text.MessageFormat; import java.util.ArrayList; import java.util.HashSet; import java.util.LinkedHashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; @@ -119,23 +120,40 @@ public class BaseDownloadActivity extends ActionBarProgressActivity { } public ItemsListBuilder getItemsBuilder() { - return getItemsBuilder(""); + return getItemsBuilder("", false); } - public ItemsListBuilder getItemsBuilder(String regionId) { - if (downloadListIndexThread.isDataPrepared()) { - return new ItemsListBuilder(getMyApplication(), regionId, downloadListIndexThread.getResourcesByRegions(), - downloadListIndexThread.getVoiceRecItems(), downloadListIndexThread.getVoiceTTSItems()); + public ItemsListBuilder getVoicePromptsBuilder() { + return getItemsBuilder("", true); + } + + public ItemsListBuilder getItemsBuilder(String regionId, boolean voicePromptsOnly) { + if (downloadListIndexThread.getResourcesLock().tryLock()) { + try { + ItemsListBuilder builder = new ItemsListBuilder(getMyApplication(), regionId, downloadListIndexThread.getResourcesByRegions(), + downloadListIndexThread.getVoiceRecItems(), downloadListIndexThread.getVoiceTTSItems()); + if (!voicePromptsOnly) { + return builder.build(); + } else { + return builder; + } + } finally { + downloadListIndexThread.getResourcesLock().unlock(); + } } else { return null; } } - public Map getIndexItemsByRegion(WorldRegion region) { - if (downloadListIndexThread.isDataPrepared()) { - return downloadListIndexThread.getResourcesByRegions().get(region); + public List getIndexItemsByRegion(WorldRegion region) { + if (downloadListIndexThread.getResourcesLock().tryLock()) { + try { + return new LinkedList<>(downloadListIndexThread.getResourcesByRegions().get(region).values()); + } finally { + downloadListIndexThread.getResourcesLock().unlock(); + } } else { - return null; + return new LinkedList<>(); } } diff --git a/OsmAnd/src/net/osmand/plus/download/DownloadIndexesThread.java b/OsmAnd/src/net/osmand/plus/download/DownloadIndexesThread.java index bc7c0ef2e2..865d530735 100644 --- a/OsmAnd/src/net/osmand/plus/download/DownloadIndexesThread.java +++ b/OsmAnd/src/net/osmand/plus/download/DownloadIndexesThread.java @@ -52,6 +52,7 @@ import java.util.Map.Entry; import java.util.Set; import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.ReentrantLock; @SuppressLint("NewApi") public class DownloadIndexesThread { @@ -73,7 +74,7 @@ public class DownloadIndexesThread { private List voiceRecItems = new LinkedList<>(); private List voiceTTSItems = new LinkedList<>(); - private boolean dataPrepared; + private final ReentrantLock resourcesLock = new ReentrantLock(); DatabaseHelper dbHelper; @@ -85,6 +86,10 @@ public class DownloadIndexesThread { dbHelper = new DatabaseHelper(app); } + public ReentrantLock getResourcesLock() { + return resourcesLock; + } + public DatabaseHelper getDbHelper() { return dbHelper; } @@ -152,10 +157,6 @@ public class DownloadIndexesThread { return itemsToUpdate; } - public boolean isDataPrepared() { - return dataPrepared; - } - public Map> getResourcesByRegions() { return resourcesByRegions; } @@ -169,45 +170,52 @@ public class DownloadIndexesThread { } private boolean prepareData(List resources) { - List resourcesInRepository; - if (resources != null) { - resourcesInRepository = resources; - } else { - resourcesInRepository = DownloadActivity.downloadListIndexThread.getCachedIndexFiles(); - } - if (resourcesInRepository == null) { - return false; - } + resourcesLock.lock(); + try { - resourcesByRegions.clear(); - voiceRecItems.clear(); - voiceTTSItems.clear(); - - for (WorldRegion region : app.getWorldRegion().getFlattenedSubregions()) { - processRegion(resourcesInRepository, false, region); - } - processRegion(resourcesInRepository, true, app.getWorldRegion()); - - final Collator collator = OsmAndCollator.primaryCollator(); - final OsmandRegions osmandRegions = app.getRegions(); - - Collections.sort(voiceRecItems, new Comparator() { - @Override - public int compare(IndexItem lhs, IndexItem rhs) { - return collator.compare(lhs.getVisibleName(app.getApplicationContext(), osmandRegions), - rhs.getVisibleName(app.getApplicationContext(), osmandRegions)); + List resourcesInRepository; + if (resources != null) { + resourcesInRepository = resources; + } else { + resourcesInRepository = DownloadActivity.downloadListIndexThread.getCachedIndexFiles(); } - }); - - Collections.sort(voiceTTSItems, new Comparator() { - @Override - public int compare(IndexItem lhs, IndexItem rhs) { - return collator.compare(lhs.getVisibleName(app.getApplicationContext(), osmandRegions), - rhs.getVisibleName(app.getApplicationContext(), osmandRegions)); + if (resourcesInRepository == null) { + return false; } - }); - return true; + resourcesByRegions.clear(); + voiceRecItems.clear(); + voiceTTSItems.clear(); + + for (WorldRegion region : app.getWorldRegion().getFlattenedSubregions()) { + processRegion(resourcesInRepository, false, region); + } + processRegion(resourcesInRepository, true, app.getWorldRegion()); + + final Collator collator = OsmAndCollator.primaryCollator(); + final OsmandRegions osmandRegions = app.getRegions(); + + Collections.sort(voiceRecItems, new Comparator() { + @Override + public int compare(IndexItem lhs, IndexItem rhs) { + return collator.compare(lhs.getVisibleName(app.getApplicationContext(), osmandRegions), + rhs.getVisibleName(app.getApplicationContext(), osmandRegions)); + } + }); + + Collections.sort(voiceTTSItems, new Comparator() { + @Override + public int compare(IndexItem lhs, IndexItem rhs) { + return collator.compare(lhs.getVisibleName(app.getApplicationContext(), osmandRegions), + rhs.getVisibleName(app.getApplicationContext(), osmandRegions)); + } + }); + + return true; + + } finally { + resourcesLock.unlock(); + } } private void processRegion(List resourcesInRepository, boolean processVoiceFiles, WorldRegion region) { @@ -531,7 +539,6 @@ public class DownloadIndexesThread { currentRunningTask.add(this); super.onPreExecute(); this.message = ctx.getString(R.string.downloading_list_indexes); - dataPrepared = false; } @Override @@ -548,7 +555,6 @@ public class DownloadIndexesThread { protected void onPostExecute(IndexFileList result) { indexFiles = result; if (indexFiles != null && uiActivity != null) { - dataPrepared = resourcesByRegions.size() > 0; boolean basemapExists = uiActivity.getMyApplication().getResourceManager().containsBasemap(); IndexItem basemap = indexFiles.getBasemap(); if (basemap != null) { diff --git a/OsmAnd/src/net/osmand/plus/download/items/ItemsListBuilder.java b/OsmAnd/src/net/osmand/plus/download/items/ItemsListBuilder.java index 45f0d60c5a..882591f151 100644 --- a/OsmAnd/src/net/osmand/plus/download/items/ItemsListBuilder.java +++ b/OsmAnd/src/net/osmand/plus/download/items/ItemsListBuilder.java @@ -1,8 +1,6 @@ package net.osmand.plus.download.items; import android.content.Context; -import android.os.Parcel; -import android.os.Parcelable; import net.osmand.PlatformUtil; import net.osmand.map.OsmandRegions; @@ -15,7 +13,6 @@ import net.osmand.plus.download.IndexItem; import net.osmand.plus.srtmplugin.SRTMPlugin; import net.osmand.util.Algorithms; -import java.io.Serializable; import java.util.Collections; import java.util.Comparator; import java.util.LinkedList; @@ -24,7 +21,7 @@ import java.util.Map; public class ItemsListBuilder { - public static final String WORLD_BASEMAP_KEY = "world_basemap.obf.zip"; + //public static final String WORLD_BASEMAP_KEY = "world_basemap.obf.zip"; public static final String WORLD_SEAMARKS_KEY = "world_seamarks_basemap.obf.zip"; private Map> resourcesByRegions; @@ -96,7 +93,7 @@ public class ItemsListBuilder { public enum VoicePromptsType { NONE, RECORDED, - TTS; + TTS } private static final org.apache.commons.logging.Log LOG = PlatformUtil.getLog(ItemsListBuilder.class); @@ -130,12 +127,12 @@ public class ItemsListBuilder { return list; } - public String getVoicePromtName(VoicePromptsType type) { + public static String getVoicePromtName(Context ctx, VoicePromptsType type) { switch (type) { case RECORDED: - return app.getResources().getString(R.string.index_name_voice); + return ctx.getResources().getString(R.string.index_name_voice); case TTS: - return app.getResources().getString(R.string.index_name_tts_voice); + return ctx.getResources().getString(R.string.index_name_tts_voice); default: return ""; } @@ -167,8 +164,8 @@ public class ItemsListBuilder { List voiceRecItems, List voiceTTSItems) { this.app = app; this.resourcesByRegions = resourcesByRegions; - this.voiceRecItems = voiceRecItems; - this.voiceTTSItems = voiceTTSItems; + this.voiceRecItems = new LinkedList<>(voiceRecItems); + this.voiceTTSItems = new LinkedList<>(voiceTTSItems); regionMapItems = new LinkedList<>(); allResourceItems = new LinkedList<>(); @@ -177,8 +174,12 @@ public class ItemsListBuilder { region = app.getWorldRegion().getRegionById(regionId); } - public boolean build() { - return obtainDataAndItems(); + public ItemsListBuilder build() { + if (obtainDataAndItems()) { + return this; + } else { + return null; + } } private boolean obtainDataAndItems() { @@ -223,7 +224,7 @@ public class ItemsListBuilder { } List regionMapArray = new LinkedList<>(); - List allResourcesArray = new LinkedList(); + List allResourcesArray = new LinkedList<>(); Context context = app.getApplicationContext(); OsmandRegions osmandRegions = app.getRegions(); diff --git a/OsmAnd/src/net/osmand/plus/download/items/RegionItemsFragment.java b/OsmAnd/src/net/osmand/plus/download/items/RegionItemsFragment.java index 7164b5ea5f..8a6b891ac7 100644 --- a/OsmAnd/src/net/osmand/plus/download/items/RegionItemsFragment.java +++ b/OsmAnd/src/net/osmand/plus/download/items/RegionItemsFragment.java @@ -73,8 +73,8 @@ public class RegionItemsFragment extends OsmandExpandableListFragment { setListView(listView); if (regionId.length() > 0) { - ItemsListBuilder builder = getDownloadActivity().getItemsBuilder(regionId); - if (builder != null && builder.build()) { + ItemsListBuilder builder = getDownloadActivity().getItemsBuilder(regionId, false); + if (builder != null) { fillRegionItemsAdapter(builder); listAdapter.notifyDataSetChanged(); expandAllGroups(); diff --git a/OsmAnd/src/net/osmand/plus/download/items/SearchItemsFragment.java b/OsmAnd/src/net/osmand/plus/download/items/SearchItemsFragment.java index 0d1e866b99..aa0f3fc01a 100644 --- a/OsmAnd/src/net/osmand/plus/download/items/SearchItemsFragment.java +++ b/OsmAnd/src/net/osmand/plus/download/items/SearchItemsFragment.java @@ -287,16 +287,11 @@ public class SearchItemsFragment extends Fragment { } for (WorldRegion region : regions) { - Map indexItems = getDownloadActivity().getIndexItemsByRegion(region); - List items = new LinkedList<>(); - if (region.getSubregions().size() > 0) { filter.add(region); } - for (IndexItem item : indexItems.values()) { - items.add(item); - } + List items = getDownloadActivity().getIndexItemsByRegion(region); if (items.size() > 1) { if (!filter.contains(region)) { filter.add(region); diff --git a/OsmAnd/src/net/osmand/plus/download/items/VoiceDialogFragment.java b/OsmAnd/src/net/osmand/plus/download/items/VoiceDialogFragment.java index 86f8be34af..5a174d707c 100644 --- a/OsmAnd/src/net/osmand/plus/download/items/VoiceDialogFragment.java +++ b/OsmAnd/src/net/osmand/plus/download/items/VoiceDialogFragment.java @@ -71,8 +71,7 @@ public class VoiceDialogFragment extends DialogFragment { VoiceItemsFragment.createInstance(voicePromptsType)).commit(); } - ItemsListBuilder builder = getDownloadActivity().getItemsBuilder(); - toolbar.setTitle(builder.getVoicePromtName(voicePromptsType)); + toolbar.setTitle(ItemsListBuilder.getVoicePromtName(getActivity(), voicePromptsType)); } ((DownloadActivity)getActivity()).registerFreeVersionBanner(view); diff --git a/OsmAnd/src/net/osmand/plus/download/items/VoiceItemsFragment.java b/OsmAnd/src/net/osmand/plus/download/items/VoiceItemsFragment.java index b0beddc599..c109f70cdf 100644 --- a/OsmAnd/src/net/osmand/plus/download/items/VoiceItemsFragment.java +++ b/OsmAnd/src/net/osmand/plus/download/items/VoiceItemsFragment.java @@ -71,7 +71,7 @@ public class VoiceItemsFragment extends OsmandExpandableListFragment { setListView(listView); if (voicePromptsType != VoicePromptsType.NONE) { - ItemsListBuilder builder = getDownloadActivity().getItemsBuilder(); + ItemsListBuilder builder = getDownloadActivity().getVoicePromptsBuilder(); if (builder != null) { fillVoiceItemsAdapter(builder); listAdapter.notifyDataSetChanged(); diff --git a/OsmAnd/src/net/osmand/plus/download/items/WorldItemsFragment.java b/OsmAnd/src/net/osmand/plus/download/items/WorldItemsFragment.java index 4a65faf675..a872869917 100644 --- a/OsmAnd/src/net/osmand/plus/download/items/WorldItemsFragment.java +++ b/OsmAnd/src/net/osmand/plus/download/items/WorldItemsFragment.java @@ -1,10 +1,20 @@ package net.osmand.plus.download.items; -import java.util.ArrayList; -import java.util.LinkedHashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; +import android.content.Context; +import android.content.res.Resources; +import android.content.res.TypedArray; +import android.graphics.drawable.Drawable; +import android.os.Bundle; +import android.support.v4.view.MenuItemCompat; +import android.util.TypedValue; +import android.view.LayoutInflater; +import android.view.Menu; +import android.view.MenuInflater; +import android.view.MenuItem; +import android.view.View; +import android.view.ViewGroup; +import android.widget.ExpandableListView; +import android.widget.TextView; import net.osmand.PlatformUtil; import net.osmand.plus.OsmandApplication; @@ -23,21 +33,11 @@ import net.osmand.plus.srtmplugin.SRTMPlugin; import org.apache.commons.logging.Log; -import android.content.Context; -import android.content.res.Resources; -import android.content.res.TypedArray; -import android.graphics.drawable.Drawable; -import android.os.Bundle; -import android.support.v4.view.MenuItemCompat; -import android.util.TypedValue; -import android.view.LayoutInflater; -import android.view.Menu; -import android.view.MenuInflater; -import android.view.MenuItem; -import android.view.View; -import android.view.ViewGroup; -import android.widget.ExpandableListView; -import android.widget.TextView; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; public class WorldItemsFragment extends OsmandExpandableListFragment { public static final String TAG = "WorldItemsFragment"; @@ -112,11 +112,11 @@ public class WorldItemsFragment extends OsmandExpandableListFragment { int unusedSubIndex = 0; List voicePromptsItems = new LinkedList<>(); if (!builder.isVoicePromptsItemsEmpty(VoicePromptsType.RECORDED)) { - voicePromptsItems.add(builder.getVoicePromtName(VoicePromptsType.RECORDED)); + voicePromptsItems.add(ItemsListBuilder.getVoicePromtName(getActivity(), VoicePromptsType.RECORDED)); voicePromptsItemsRecordedSubIndex = unusedSubIndex++; } if (!builder.isVoicePromptsItemsEmpty(VoicePromptsType.TTS)) { - voicePromptsItems.add(builder.getVoicePromtName(VoicePromptsType.TTS)); + voicePromptsItems.add(ItemsListBuilder.getVoicePromtName(getActivity(), VoicePromptsType.TTS)); voicePromptsItemsTTSSubIndex = unusedSubIndex; } if (!voicePromptsItems.isEmpty()) { @@ -187,8 +187,8 @@ public class WorldItemsFragment extends OsmandExpandableListFragment { } public void onCategorizationFinished() { - ItemsListBuilder builder = getDownloadActivity().getItemsBuilder(); - if (builder != null && builder.build()) { + ItemsListBuilder builder = getDownloadActivity().getItemsBuilder(); + if (builder != null) { fillWorldItemsAdapter(builder); listAdapter.notifyDataSetChanged(); expandAllGroups();