♻️ refactor: offline http cache #723
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
wssgcg1213
commented
Oct 9, 2021
•
wssgcg1213
commented
- 解决大量访问缓存时已经 open 的连接未 close/abort 而导致大量 CLOSE_WAIT 继而消耗完文件句柄的问题:
- 原因: 原先 HttpCache 在创建请求对象 (HttpClientRequest) 的时候, 直接使用了 http_impl 中的 HttpClientRequest, 其在 openUrl 的时候就会创建 tcp connection, 完成 dns lookup + tcp 三次握手过程，其后如果命中缓存, 则会进入缓存逻辑, 所以此 tcp connection 未被关闭或取消 (close/abort), 从而导致连接泄露, 大量请求后表现为句柄数耗尽。
- 本方案: 在创建请求对象时直接创建 ProxyHttpClientRequest, 而不直接去创建 backend 的真实 HttpClientRequest, 在命中缓存情况下直接返回, 若未命中则再去创建和代理真实请求。
- 额外带来的好处: 支持真正的离线缓存, 不需要提前创建 dns 查询和 tcp connection, 若全部资源在缓存中则无需网络连接即可完成加载
- 增加 blob 文件尺寸的校验, 避免被恶意替换或者网络中断导致的文件缺损
answershuto
reviewed
Oct 11, 2021
answershuto
reviewed
Oct 12, 2021
| final int hash = key.hashCode; | ||
| final Directory cacheDirectory = await getCacheDirectory(); | ||
| HttpCacheObject cacheObject = HttpCacheObject(key, cacheDirectory.path, hash: hash, origin: _origin); | ||
|
|
||
| await cacheObject.read(); |
There was a problem hiding this comment.
这两边的改动差了一个 cacheObject.read(); 的执行，原本是有问题还是说内存中的 cacheObject 原本不需要 read？
There was a problem hiding this comment.
下面有一行 read 去掉了, 统一到这里来; 原来的处理对于非缓存的情况会 read 两次, 虽然 read 函数里面有 valid 校验, 不会导致实际 io 两次, 但是改成这样更合理一些
|
没有看到新增的测试用例 |
answershuto
approved these changes
Oct 12, 2021
之前的测试已经覆盖到修改的部分代码了, 这个问题在 macos 下不会有异常的表现, 只在 iOS/Android 上有异常, 所以修复之后也无法在 macos 上测试验证, 通过 netstats 等工具可以观察到打开的连接, 但是没办法量化, 所以已经单独做过测试之后才提交 |
temper357
approved these changes
Oct 12, 2021
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.