Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.尝试修复由于DownloadRunnable在FetchDataTask中出现异常时,未将全部DownloadLaunchRunnal… #769

Closed
wants to merge 1 commit into from

Conversation

BigExcalibur
Copy link

@BigExcalibur BigExcalibur commented Sep 22, 2017

1.尝试修复由于DownloadRunnable在FetchDataTask中出现异常时,未将全部DownloadLaunchRunnable创建的全部线程关闭的问题。原代码的意图是忽略该异常并重新再DownloadLaunchRunnable中重新创建,经过测试后发现,该异常会一直存在,因此没有意义,并且最终会导致socketTimeout异常,具体原因不明。

2.尝试修复由于DownloadLaunchRunnable因为子线程上抛异常时退出,handlerThread死亡后无法回调error,导致DownloadTask状态异常而重新下载的问题。原代码中因为pause的flag而忽略异常的判断会导致异常无法正确回调,故增加判断,保证onerror能正确回调并回收线程。

具体代码可能有些不合适,还请大佬验证。修改后的代码经过测试,目前可以解决在恶劣网络状况下会频繁重新下载的问题。

…e创建的全部线程关闭的问题。原代码的意图是忽略该异常并重新再DownloadLaunchRunnable中重新创建,经过测试后发现,该异常会一直存在,因此没有意义,并且最终会导致socketTimeout异常,具体原因不明。

2.尝试修复由于DownloadLaunchRunnable因为子线程上抛异常时退出,handlerThread死亡后无法回调error,导致DownloadTask状态异常而重新下载的问题。原代码中因为pause的flag而忽略异常的判断会导致异常无法正确回调,故增加判断,保证onerror能正确回调并回收线程。

具体代码可能有些不合适,还请大佬验证。
@BigExcalibur
Copy link
Author

image

@Jacksgong
Copy link
Collaborator

Jacksgong commented Sep 22, 2017

感谢PR。。不过我有几点疑惑:

1. 原代码的意图是忽略该异常并重新再DownloadLaunchRunnable中重新创建,经过测试后发现,该异常会一直存在,因此没有意义

这个原代码在FetchDataTask遇到异常的时候,抛到DownloadRunnable然后往上抛到DownloadLaunchRunnable#onError后,会直接通过每个任务的DownloadRunnable#discard,结束每个运行中的任务。因此不存在描述的问题。。其中FetchDataTask有重试机会,这个重试机会由DownloadTask#setAutoRetryTimes控制。

2.尝试修复由于DownloadLaunchRunnable因为子线程上抛异常时退出,handlerThread死亡后无法回调error,导致DownloadTask状态异常而重新下载的问题。原代码中因为pause的flag而忽略异常的判断会导致异常无法正确回调,故增加判断,保证onerror能正确回调并回收线程。

这个我不是很理解,在DownloadLaunchRunnable#fetchWithMultipleConnection中会阻塞住当前线程,直到所有子任务所在线程都完成(具体可以参见(ExecutorService#invokeAll)),为什么会有子线程上抛异常时,handlerthread死亡的问题?

@BigExcalibur
Copy link
Author

BigExcalibur commented Sep 22, 2017

@Jacksgong
是这样的,由于对大佬你的代码我还没有完全看明白,尤其是自动重试那一块,目前我只是带着可以解决目前我遇到的问题而尝试的修改。

目前比较奇怪的是抛出来的异常,error message为null,我定位到的抛出异常的位置为FetchDataTask#run的inputStream.read(buff),但是上层似乎是直接当成SocketTimeout异常捕获了,并且从日志中得出DownloadLaunchRunnable的lifecycle over调用了。
我使用原库时,断点续传失败时,出现了的日志为
-687888077 status is[3](not finish) & but not in the pool
说明lifecycle over调用以后,线程死亡后并未回收也未从内存中删除,也未回调onerror方法将status同步。

如果无法模拟恶劣网络状态的话,我建议你可以采用手动随机抛异常的方式尝试模拟一下。

@BigExcalibur
Copy link
Author

image

偶尔还是会出现重新下载的问题,虽然几率比以前小很多了,但是还是会出现.. 希望大佬能够提供一些思路。

@Jacksgong
Copy link
Collaborator

嗯,首先需要确定下代码是否在master最新代码上对吗(1.6.5),因为1.5.0之后有一个版本这块有点问题,后来很快紧急修复了。

为了便于描述,下面将DownloadLaunchRunnable所在线程,称为“宿主线程”,FetchDataTask所在线程,称为“子线程”。

I. 目前比较奇怪的是抛出来的异常,error message为null,我定位到的抛出异常的位置为FetchDataTask#run的inputStream.read(buff),但是上层似乎是直接当成SocketTimeout异常捕获了

说明就是一个写入时的SocketTimeoutExecption的类型已经可以说明问题时,因此这个Api就没有再带具体message,这个在设计规范上是允许的)

II. 并且从日子中得出DownloadLaunchRunnable的lifecycle over调用了。

这个我不是很理解,DownloadLaunchRunnable的lifecycle over指的是什么?

III. 我使用原库时,断点续传失败时,出现了的日志为-687888077 status is[3](not finish) & but not in the pool

首先,出现这个日志,是一个挺严重的问题,其说明了:

任务状态还在progress(3)下载中的状态,但是在池子中却没有该任务存在,而这个只有可能是:

任务宿主线程已经结束了并且其回调的handlerThreadLooperquit了(这个quit,只有在回调了结束,并且任务状态被刷新为结束,才会被促发,参看DownloadStatusCallback#handleMessage中最后一段的代码); 但是下载状态还停留在下载中。显然这里是相互矛盾的。这个需要进一步分析:

  1. 状态刷新失败? => 不存在 -> 压根就没有调用到状态刷新代码? => 没有调到状态结束的代码,looper怎么可能被退出,不成立 -> 状态流存在问题,时序问题,导致状态错乱 => 结束是在DownloadLaunchRunnable#onComplete统一处理的,只有在所有子线程都结束了,才会回调结束,因此不存在该问题。
  2. 线程被提前结束了并且分块下载回调的looper被提前退出了?=> 这个显然不成立,有阻塞与任务结束任务时才退出保障。

出现这个问题的情况,是在master的最新分支吗,根据理论分析应该存在这个情况,如果可以复现,可能只能通过完整的日志进行分析了(其中要包含上次与该次的完整日志)。

IV. 偶尔还是会出现重新下载的问题,虽然几率比以前小很多了,但是还是会出现..

出现这个问题,由于是低概率事件,可以通过日志分析(可以借助我之前开源的okcat)。

我们考虑缩小范围去看待这个问题:

  1. 首先停止是暂停或者异常
  2. 如果是分块下载,因为有多个FetchDataTask并且DownloadStatusCallback通过单独的handlerThread进行回调,不是直接回调
  3. 通过FileDownloadUtils#isBreakpointAvailable输出的原因,再根据其输出的原因往上找根本原因
  4. 记录多个存在问题的日志,对比原因

@BigExcalibur
Copy link
Author

感谢大佬在百忙之中抽出时间来解答。

首先我确定使用的库是最新的代码,直接从master分支fork过来的。

第一个问题已经了解了。

第二个问题是我的表述有些问题,实际指的是
DownloadTaskHunter: filedownloader:lifecycle:over com.liulishuo.filedownloader.DownloadTaskHunter@aab28d6 by -1
这段日志,因为DownloadTaskHunter这个类目前的作用我还不太明了,还请大佬能够指明一下。

第三个问题
日志的话 #763 这里有我前段时间分析的整个流程和日志。
我目前还没有完全确认是handler未回调onError的问题还是onProgress回调覆盖了error状态的原因,或者两者都有,按照我测试的情况是二者都有。

目前我修改的部分的思路是当异常抛出时,杀死宿主线程和所有子线程来保证不会回调onProgress,但是似乎不能完全奏效,低概率依然会出现,因为我们公司的网络状况确实极差。(但是或许也能让代码能够暴露出一些平时见不到的问题 笑~~ :-D)

增加deleteThread的接口即是为了实现这个目的。

另外一个是在DownloadLaunchRunnable#onError中增加了判断

 if (paused && (model.getStatus() == FileDownloadStatus.error || model.getStatus() == FileDownloadStatus.paused)) 

保证即使回调onProgress把onError状态刷新掉了,还是能够回调onError接口处理异常。不过不能完全奏效。

第四个问题
大佬你的okcat这个项目我前几天看到了,准备去拜读一下。

低概率事件指的是经过Pr修改后的代码,如果是master分支的源代码,出现这个问题的几率非常高。几乎每次都会重新下载。

关于为什么出现,我在上述第三个问题已经阐明了,只是因为异步线程流程无法确认导致目前我还不能完全定位,后续我会尝试精确跟踪每个线程的流程和生命周期,尝试发现问题。由于问题不是稳定复现,所以调试难度较大,后续的问题我还会继续跟进,直到这个问题解决为止。

@Jacksgong
Copy link
Collaborator

Jacksgong commented Oct 12, 2017

@BigExcalibur 不好意思,由于我自己也不满意这个库,有些地方复杂度太高了,整个核心库不够单一,单元测试覆盖极少等问题,导致PR很少,线上容易存在边界问题难以发现,因此这段事件我一直在重新编写新库,耽搁了时间,望见谅。


I. 第二个问题这段日志,因为DownloadTaskHunter这个类目前的作用我还不太明了。

DownloadTaskHunter 是在主进程中处理每个任务的,可能你把他理解为DownloadTaskHandler会更好理解些,每个Task都有自己的DownloadTaskHunter,所有任务的事务,如启动、状态回调、暂停处理等等,都是通过DownloadTaskHunter进行处理的。

filedownloader:lifecycle:over 这个日志是在任务结束时,DownloadTaskHunter在主进程收到了该任务的结束的状态(pause\error\completed\warn)时输出。

II. 第三个问题我目前还没有完全确认是handler未回调onError的问题还是onProgress回调覆盖了error状态的原因

我检查了下确实有可能 onErroronPaused与其他回调确实存在可能被覆盖的问题,在高并发的DownloadStatusCallback回调中,我一会儿用相对较轻的方式解决,然后提交一个commit,你可以看下,应该可以避免这个问题。

III. 第四个问题

谢谢支持哈~

@Jacksgong Jacksgong closed this in ca2f276 Oct 12, 2017
@Jacksgong Jacksgong modified the milestones: 1.6.7, 1.6.8 Oct 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants