{"action":"created","comment":{"url":"https://api.github.com/repos/bnsantos/TileView/pulls/comments/53793238","id":53793238,"diff_hunk":"@@ -0,0 +1,151 @@\n+package com.qozix.tileview.tiles;\n+\n+import android.os.Process;\n+\n+import java.lang.ref.WeakReference;\n+import java.util.HashMap;\n+import java.util.LinkedList;\n+import java.util.concurrent.BlockingQueue;\n+import java.util.concurrent.Future;\n+import java.util.concurrent.LinkedBlockingDeque;\n+import java.util.concurrent.ThreadPoolExecutor;\n+import java.util.concurrent.TimeUnit;\n+\n+public class TileRenderPoolManager {\n+\n+  private static final int KEEP_ALIVE_TIME = 1;\n+  private static final TimeUnit KEEP_ALIVE_TIME_UNIT = TimeUnit.SECONDS;\n+\n+  private static final int CORE_POOL_SIZE = Runtime.getRuntime().availableProcessors();\n+  private static final int MAXIMUM_POOL_SIZE = Runtime.getRuntime().availableProcessors();\n+\n+  private final TileRenderThreadPoolExecutor mTileRenderThreadPoolExecutor;\n+  private final BlockingQueue<Runnable> mRunnableLinkedBlockingDeque = new LinkedBlockingDeque<>();\n+  private final HashMap<Future, TileRenderRunnable> mFutureTileRenderRunnableHashMap = new HashMap<>();\n+  private WeakReference<TileCanvasViewGroup> mTileCanvasViewGroupWeakReference;\n+\n+  public TileRenderPoolManager() {\n+    mTileRenderThreadPoolExecutor = new TileRenderThreadPoolExecutor(\n+      CORE_POOL_SIZE,\n+      MAXIMUM_POOL_SIZE,\n+      KEEP_ALIVE_TIME,\n+      KEEP_ALIVE_TIME_UNIT,\n+      mRunnableLinkedBlockingDeque\n+    );\n+  }\n+\n+  public void cancel() {\n+    synchronized( this ) {\n+      if( mRunnableLinkedBlockingDeque.size() > 0 || mTileRenderThreadPoolExecutor.getActiveCount() > 0 ) {\n+        TileCanvasViewGroup tileCanvasViewGroup = mTileCanvasViewGroupWeakReference.get();\n+        if( tileCanvasViewGroup != null ) {\n+          tileCanvasViewGroup.onRenderTaskCancelled();\n+        }\n+      }\n+      for( Future future : mFutureTileRenderRunnableHashMap.keySet() ) {\n+        if( !future.isDone() ) {\n+          if( mFutureTileRenderRunnableHashMap.containsKey( future ) ) {\n+            TileRenderRunnable tileRenderRunnable = mFutureTileRenderRunnableHashMap.get( future );\n+            tileRenderRunnable.cancel();\n+          }\n+          future.cancel( true );\n+        }\n+      }\n+      mRunnableLinkedBlockingDeque.clear();\n+      mFutureTileRenderRunnableHashMap.clear();\n+    }\n+  }\n+\n+  public void shutdown() {\n+    cancel();\n+    mTileRenderThreadPoolExecutor.shutdownNow();\n+  }\n+\n+  public void queue( TileCanvasViewGroup tileCanvasViewGroup, LinkedList<Tile> tileLinkedList ) {\n+    if( tileLinkedList != null && tileLinkedList.size() > 0 ) {\n+      mTileCanvasViewGroupWeakReference = new WeakReference<>( tileCanvasViewGroup );\n+      tileCanvasViewGroup.onRenderTaskPreExecute();","path":"tileview/src/main/java/com/qozix/tileview/tiles/TileRenderPoolManager.java","position":67,"original_position":67,"commit_id":"2f2e93c1975786423a33c02ab984dc709e16669f","original_commit_id":"2f2e93c1975786423a33c02ab984dc709e16669f","user":{"login":"moagrius","id":701344,"avatar_url":"https://avatars.githubusercontent.com/u/701344?v=3","gravatar_id":"","url":"https://api.github.com/users/moagrius","html_url":"https://github.com/moagrius","followers_url":"https://api.github.com/users/moagrius/followers","following_url":"https://api.github.com/users/moagrius/following{/other_user}","gists_url":"https://api.github.com/users/moagrius/gists{/gist_id}","starred_url":"https://api.github.com/users/moagrius/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/moagrius/subscriptions","organizations_url":"https://api.github.com/users/moagrius/orgs","repos_url":"https://api.github.com/users/moagrius/repos","events_url":"https://api.github.com/users/moagrius/events{/privacy}","received_events_url":"https://api.github.com/users/moagrius/received_events","type":"User","site_admin":false},"body":"i don't see a problem doing that; then again, i don't see much difference either way - do you see value in moving it?","created_at":"2016-02-23T15:16:12Z","updated_at":"2016-02-23T15:16:12Z","html_url":"https://github.com/bnsantos/TileView/pull/1#discussion_r53793238","pull_request_url":"https://api.github.com/repos/bnsantos/TileView/pulls/1","_links":{"self":{"href":"https://api.github.com/repos/bnsantos/TileView/pulls/comments/53793238"},"html":{"href":"https://github.com/bnsantos/TileView/pull/1#discussion_r53793238"},"pull_request":{"href":"https://api.github.com/repos/bnsantos/TileView/pulls/1"}}},"pull_request":{"url":"https://api.github.com/repos/bnsantos/TileView/pulls/1","id":60190303,"html_url":"https://github.com/bnsantos/TileView/pull/1","diff_url":"https://github.com/bnsantos/TileView/pull/1.diff","patch_url":"https://github.com/bnsantos/TileView/pull/1.patch","issue_url":"https://api.github.com/repos/bnsantos/TileView/issues/1","number":1,"state":"open","locked":false,"title":" Thread interrupt race","user":{"login":"moagrius","id":701344,"avatar_url":"https://avatars.githubusercontent.com/u/701344?v=3","gravatar_id":"","url":"https://api.github.com/users/moagrius","html_url":"https://github.com/moagrius","followers_url":"https://api.github.com/users/moagrius/followers","following_url":"https://api.github.com/users/moagrius/following{/other_user}","gists_url":"https://api.github.com/users/moagrius/gists{/gist_id}","starred_url":"https://api.github.com/users/moagrius/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/moagrius/subscriptions","organizations_url":"https://api.github.com/users/moagrius/orgs","repos_url":"https://api.github.com/users/moagrius/repos","events_url":"https://api.github.com/users/moagrius/events{/privacy}","received_events_url":"https://api.github.com/users/moagrius/received_events","type":"User","site_admin":false},"body":"@bnsantos @peterLaurence here's the PR for the changes that prevent the empty or incomplete tiles from being drawn.\r\n\r\nThe `skia: ---- read threw an exception` messages will continue to show, AFAIK there's nothing we can do about it - somewhere in the framework they're catching the `Exception` and printing the stack trace, which is what we're seeing - since it's caught by them, we can't catch (and therefore, suppress) it ourselves.  Sucks, but as long as we're interrupting the thread to stop downloads prematurely (which I believe is the best practice here, since we don't want to finish downloads we don't need), it's going to get printed.\r\n\r\nGenerally speaking, I've changed the name of the class and re-arranged things in order to help me grok your original strategy, but really not much behavior has changed.  I especially wanted to make the logic of the `run` method (where most of what we were trying to do was) very obvious.  Also, the main class was called \"Executor\" but it didn't extend executor, and instead used an instance variable that did, also named similarly - I changed it to `TileRenderPoolManager` to make it more clear, and the actual `Executor` member still has that component in its name.  @bnsantos if you want to revert to any of the old variables names, it's not a big deal.\r\n\r\nTo note:\r\n\r\n1.  The major functional change here - what's preventing incomplete tiles from being delivered - is to set a \"canceled\" indication on the `Runnable` attached to the `Future`: https://github.com/bnsantos/TileView/compare/master...bnsantos:thread-interrupt-race?expand=1#diff-9c5513cafc3e377889e8b5837465bbe7R49.  By checking `cancelled` state in the `Runnable.run` method, it successfully exits before adding the tile - even though we were checking `thread.isInterrupted` before, which should do the same thing in theory.\r\n\r\n2.  I originally was overriding the `Future` used by the `ThreadPoolExecutor`, but to keep things a little lighter I just associate each `Future` with the `Runnable` it takes using a `HashMap`.  This happens on line 74: https://github.com/bnsantos/TileView/compare/master...bnsantos:thread-interrupt-race?expand=1#diff-9c5513cafc3e377889e8b5837465bbe7R74.  \r\n\r\n3.  Since we have the `HashMap` now anyway, I didn't see any need for the `List` of `Future`s we were maintaining, since a list now exists in the `keySet` of the `HashMap`.  However, I did notice we were using a `BlockingQueue` (actually a `LinkedBlockingDeque`) for the `Future` list previously - the `keySet` will not have blocking behavior - do you anticipate any problems by _not_ using a `BlockingQueue` here?\r\n\r\n4.  If the tile does get generated (it makes it far enough through the `run` method's conditions that it's passed to the decoder), but during the time it took to decode, conditions change enough that we no longer want it, I added a call to `tile.destroy` to reclaim the memory.\r\n\r\n5.  `TileRenderRunnable.mCancelled` is `volatile`, per @peterLaurence suggestion, which I agree with.  The alternative would be to synchronize the `cancel` method.  IIUC, `volatile` is more appropriate here and accomplishes everything that `synchronize` would do - any idea otherwise?  Does this look correct, complete and reliable?\r\n\r\n6.  I'm generally not a fan of nested instance classes (nested static classes are fine) - what do you guys think about the `ThreadPoolExecutor` subclass being an instance class?  If you don't see any potential problems, then we can leave it, otherwise we could either A) make it static, and `WeakReferences` for the instance members it references, or B) get rid of it entirely, and have the main class subclass `ThreadPoolExecutor` itself.\r\n\r\n7.  General question - why is `cancel` on the main class (what I now call `TileRenderPoolManager `) synchronized?  Is that ever called from a thread other than the UI thread?\r\n\r\nI'm 100% open to any feedback and expect to make changes, so feel free to comment or question anything you see.\r\n\r\nThanks,\r\nMike","created_at":"2016-02-22T15:31:20Z","updated_at":"2016-02-23T15:16:12Z","closed_at":null,"merged_at":null,"merge_commit_sha":"de0254a3b1a8f0c69873f171cf254cc9b011dc43","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/bnsantos/TileView/pulls/1/commits","review_comments_url":"https://api.github.com/repos/bnsantos/TileView/pulls/1/comments","review_comment_url":"https://api.github.com/repos/bnsantos/TileView/pulls/comments{/number}","comments_url":"https://api.github.com/repos/bnsantos/TileView/issues/1/comments","statuses_url":"https://api.github.com/repos/bnsantos/TileView/statuses/2f2e93c1975786423a33c02ab984dc709e16669f","head":{"label":"bnsantos:thread-interrupt-race","ref":"thread-interrupt-race","sha":"2f2e93c1975786423a33c02ab984dc709e16669f","user":{"login":"bnsantos","id":4581581,"avatar_url":"https://avatars.githubusercontent.com/u/4581581?v=3","gravatar_id":"","url":"https://api.github.com/users/bnsantos","html_url":"https://github.com/bnsantos","followers_url":"https://api.github.com/users/bnsantos/followers","following_url":"https://api.github.com/users/bnsantos/following{/other_user}","gists_url":"https://api.github.com/users/bnsantos/gists{/gist_id}","starred_url":"https://api.github.com/users/bnsantos/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bnsantos/subscriptions","organizations_url":"https://api.github.com/users/bnsantos/orgs","repos_url":"https://api.github.com/users/bnsantos/repos","events_url":"https://api.github.com/users/bnsantos/events{/privacy}","received_events_url":"https://api.github.com/users/bnsantos/received_events","type":"User","site_admin":false},"repo":{"id":51532983,"name":"TileView","full_name":"bnsantos/TileView","owner":{"login":"bnsantos","id":4581581,"avatar_url":"https://avatars.githubusercontent.com/u/4581581?v=3","gravatar_id":"","url":"https://api.github.com/users/bnsantos","html_url":"https://github.com/bnsantos","followers_url":"https://api.github.com/users/bnsantos/followers","following_url":"https://api.github.com/users/bnsantos/following{/other_user}","gists_url":"https://api.github.com/users/bnsantos/gists{/gist_id}","starred_url":"https://api.github.com/users/bnsantos/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bnsantos/subscriptions","organizations_url":"https://api.github.com/users/bnsantos/orgs","repos_url":"https://api.github.com/users/bnsantos/repos","events_url":"https://api.github.com/users/bnsantos/events{/privacy}","received_events_url":"https://api.github.com/users/bnsantos/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/bnsantos/TileView","description":"The TileView widget is a subclass of android.view.ViewGroup that asynchronously displays tile-based images, with pan and zoom functionality, and features like markers, hotspots, paths, multiple levels of detail, and arbitrary coordinate systems.","fork":true,"url":"https://api.github.com/repos/bnsantos/TileView","forks_url":"https://api.github.com/repos/bnsantos/TileView/forks","keys_url":"https://api.github.com/repos/bnsantos/TileView/keys{/key_id}","collaborators_url":"https://api.github.com/repos/bnsantos/TileView/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/bnsantos/TileView/teams","hooks_url":"https://api.github.com/repos/bnsantos/TileView/hooks","issue_events_url":"https://api.github.com/repos/bnsantos/TileView/issues/events{/number}","events_url":"https://api.github.com/repos/bnsantos/TileView/events","assignees_url":"https://api.github.com/repos/bnsantos/TileView/assignees{/user}","branches_url":"https://api.github.com/repos/bnsantos/TileView/branches{/branch}","tags_url":"https://api.github.com/repos/bnsantos/TileView/tags","blobs_url":"https://api.github.com/repos/bnsantos/TileView/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/bnsantos/TileView/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/bnsantos/TileView/git/refs{/sha}","trees_url":"https://api.github.com/repos/bnsantos/TileView/git/trees{/sha}","statuses_url":"https://api.github.com/repos/bnsantos/TileView/statuses/{sha}","languages_url":"https://api.github.com/repos/bnsantos/TileView/languages","stargazers_url":"https://api.github.com/repos/bnsantos/TileView/stargazers","contributors_url":"https://api.github.com/repos/bnsantos/TileView/contributors","subscribers_url":"https://api.github.com/repos/bnsantos/TileView/subscribers","subscription_url":"https://api.github.com/repos/bnsantos/TileView/subscription","commits_url":"https://api.github.com/repos/bnsantos/TileView/commits{/sha}","git_commits_url":"https://api.github.com/repos/bnsantos/TileView/git/commits{/sha}","comments_url":"https://api.github.com/repos/bnsantos/TileView/comments{/number}","issue_comment_url":"https://api.github.com/repos/bnsantos/TileView/issues/comments{/number}","contents_url":"https://api.github.com/repos/bnsantos/TileView/contents/{+path}","compare_url":"https://api.github.com/repos/bnsantos/TileView/compare/{base}...{head}","merges_url":"https://api.github.com/repos/bnsantos/TileView/merges","archive_url":"https://api.github.com/repos/bnsantos/TileView/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/bnsantos/TileView/downloads","issues_url":"https://api.github.com/repos/bnsantos/TileView/issues{/number}","pulls_url":"https://api.github.com/repos/bnsantos/TileView/pulls{/number}","milestones_url":"https://api.github.com/repos/bnsantos/TileView/milestones{/number}","notifications_url":"https://api.github.com/repos/bnsantos/TileView/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/bnsantos/TileView/labels{/name}","releases_url":"https://api.github.com/repos/bnsantos/TileView/releases{/id}","deployments_url":"https://api.github.com/repos/bnsantos/TileView/deployments","created_at":"2016-02-11T17:38:12Z","updated_at":"2016-02-22T12:36:49Z","pushed_at":"2016-02-22T15:31:20Z","git_url":"git://github.com/bnsantos/TileView.git","ssh_url":"git@github.com:bnsantos/TileView.git","clone_url":"https://github.com/bnsantos/TileView.git","svn_url":"https://github.com/bnsantos/TileView","homepage":"http://moagrius.github.io/TileView/","size":148120,"stargazers_count":1,"watchers_count":1,"language":"Java","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":1,"forks":1,"open_issues":1,"watchers":1,"default_branch":"master"}},"base":{"label":"bnsantos:master","ref":"master","sha":"e0452a392e450157bebb2ce010f539356515e3d3","user":{"login":"bnsantos","id":4581581,"avatar_url":"https://avatars.githubusercontent.com/u/4581581?v=3","gravatar_id":"","url":"https://api.github.com/users/bnsantos","html_url":"https://github.com/bnsantos","followers_url":"https://api.github.com/users/bnsantos/followers","following_url":"https://api.github.com/users/bnsantos/following{/other_user}","gists_url":"https://api.github.com/users/bnsantos/gists{/gist_id}","starred_url":"https://api.github.com/users/bnsantos/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bnsantos/subscriptions","organizations_url":"https://api.github.com/users/bnsantos/orgs","repos_url":"https://api.github.com/users/bnsantos/repos","events_url":"https://api.github.com/users/bnsantos/events{/privacy}","received_events_url":"https://api.github.com/users/bnsantos/received_events","type":"User","site_admin":false},"repo":{"id":51532983,"name":"TileView","full_name":"bnsantos/TileView","owner":{"login":"bnsantos","id":4581581,"avatar_url":"https://avatars.githubusercontent.com/u/4581581?v=3","gravatar_id":"","url":"https://api.github.com/users/bnsantos","html_url":"https://github.com/bnsantos","followers_url":"https://api.github.com/users/bnsantos/followers","following_url":"https://api.github.com/users/bnsantos/following{/other_user}","gists_url":"https://api.github.com/users/bnsantos/gists{/gist_id}","starred_url":"https://api.github.com/users/bnsantos/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bnsantos/subscriptions","organizations_url":"https://api.github.com/users/bnsantos/orgs","repos_url":"https://api.github.com/users/bnsantos/repos","events_url":"https://api.github.com/users/bnsantos/events{/privacy}","received_events_url":"https://api.github.com/users/bnsantos/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/bnsantos/TileView","description":"The TileView widget is a subclass of android.view.ViewGroup that asynchronously displays tile-based images, with pan and zoom functionality, and features like markers, hotspots, paths, multiple levels of detail, and arbitrary coordinate systems.","fork":true,"url":"https://api.github.com/repos/bnsantos/TileView","forks_url":"https://api.github.com/repos/bnsantos/TileView/forks","keys_url":"https://api.github.com/repos/bnsantos/TileView/keys{/key_id}","collaborators_url":"https://api.github.com/repos/bnsantos/TileView/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/bnsantos/TileView/teams","hooks_url":"https://api.github.com/repos/bnsantos/TileView/hooks","issue_events_url":"https://api.github.com/repos/bnsantos/TileView/issues/events{/number}","events_url":"https://api.github.com/repos/bnsantos/TileView/events","assignees_url":"https://api.github.com/repos/bnsantos/TileView/assignees{/user}","branches_url":"https://api.github.com/repos/bnsantos/TileView/branches{/branch}","tags_url":"https://api.github.com/repos/bnsantos/TileView/tags","blobs_url":"https://api.github.com/repos/bnsantos/TileView/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/bnsantos/TileView/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/bnsantos/TileView/git/refs{/sha}","trees_url":"https://api.github.com/repos/bnsantos/TileView/git/trees{/sha}","statuses_url":"https://api.github.com/repos/bnsantos/TileView/statuses/{sha}","languages_url":"https://api.github.com/repos/bnsantos/TileView/languages","stargazers_url":"https://api.github.com/repos/bnsantos/TileView/stargazers","contributors_url":"https://api.github.com/repos/bnsantos/TileView/contributors","subscribers_url":"https://api.github.com/repos/bnsantos/TileView/subscribers","subscription_url":"https://api.github.com/repos/bnsantos/TileView/subscription","commits_url":"https://api.github.com/repos/bnsantos/TileView/commits{/sha}","git_commits_url":"https://api.github.com/repos/bnsantos/TileView/git/commits{/sha}","comments_url":"https://api.github.com/repos/bnsantos/TileView/comments{/number}","issue_comment_url":"https://api.github.com/repos/bnsantos/TileView/issues/comments{/number}","contents_url":"https://api.github.com/repos/bnsantos/TileView/contents/{+path}","compare_url":"https://api.github.com/repos/bnsantos/TileView/compare/{base}...{head}","merges_url":"https://api.github.com/repos/bnsantos/TileView/merges","archive_url":"https://api.github.com/repos/bnsantos/TileView/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/bnsantos/TileView/downloads","issues_url":"https://api.github.com/repos/bnsantos/TileView/issues{/number}","pulls_url":"https://api.github.com/repos/bnsantos/TileView/pulls{/number}","milestones_url":"https://api.github.com/repos/bnsantos/TileView/milestones{/number}","notifications_url":"https://api.github.com/repos/bnsantos/TileView/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/bnsantos/TileView/labels{/name}","releases_url":"https://api.github.com/repos/bnsantos/TileView/releases{/id}","deployments_url":"https://api.github.com/repos/bnsantos/TileView/deployments","created_at":"2016-02-11T17:38:12Z","updated_at":"2016-02-22T12:36:49Z","pushed_at":"2016-02-22T15:31:20Z","git_url":"git://github.com/bnsantos/TileView.git","ssh_url":"git@github.com:bnsantos/TileView.git","clone_url":"https://github.com/bnsantos/TileView.git","svn_url":"https://github.com/bnsantos/TileView","homepage":"http://moagrius.github.io/TileView/","size":148120,"stargazers_count":1,"watchers_count":1,"language":"Java","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":1,"forks":1,"open_issues":1,"watchers":1,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/bnsantos/TileView/pulls/1"},"html":{"href":"https://github.com/bnsantos/TileView/pull/1"},"issue":{"href":"https://api.github.com/repos/bnsantos/TileView/issues/1"},"comments":{"href":"https://api.github.com/repos/bnsantos/TileView/issues/1/comments"},"review_comments":{"href":"https://api.github.com/repos/bnsantos/TileView/pulls/1/comments"},"review_comment":{"href":"https://api.github.com/repos/bnsantos/TileView/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/bnsantos/TileView/pulls/1/commits"},"statuses":{"href":"https://api.github.com/repos/bnsantos/TileView/statuses/2f2e93c1975786423a33c02ab984dc709e16669f"}}}}