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

修复各种扩展中taskIdentifier的线程安全问题 它在不同线程中调用可能出现崩溃问题 #1276

Merged
merged 2 commits into from
Sep 12, 2019

Conversation

lixiang1994
Copy link
Contributor

可用类似代码模拟场景复现, 该问题是通过Bugly收集到的
image

@onevcat
Copy link
Owner

onevcat commented Sep 11, 2019

These extension methods belong to the UI part, and they are not designed as thread-safe methods.

Similar to the UIKit things, you need to make sure to call them from the main thread to keep all accessing safe.

@lixiang1994
Copy link
Contributor Author

onShouldApply闭包中使用了taskIdentifier, onShouldApply可能在任何线程被调用

@lixiang1994
Copy link
Contributor Author

或者需要确保onShouldApply只在主线程内调用, 你更倾向于哪一种?

@lixiang1994
Copy link
Contributor Author

@onevcat ??? 怎么说

@onevcat
Copy link
Owner

onevcat commented Sep 12, 2019

It makes sense. But several concerns:

  1. Why you didn't add lock in the setter of ImageView and NSButton extensions. Should we also need lock it in the identifier setter?
  2. Can you remove the DevelopmentTeam changing from the PR?

@lixiang1994
Copy link
Contributor Author

objc_getAssociatedObjectobjc_setAssociatedObject是线程安全的.
AssociationsManager 通过持有一个自旋锁spinlock_t保证对AssociationsHashMap的操作是线程安全的, 即每次只会有一个线程对AssociationsHashMap进行操作.
我们的问题是Box中的value并不是线程安全的, 所以只需要在value相关操作时增加锁就可以了.

@onevcat onevcat merged commit 9e0ca25 into onevcat:master Sep 12, 2019
@lixiang1994
Copy link
Contributor Author

🐱喵喵喵~

@onevcat onevcat mentioned this pull request Sep 25, 2019
3 tasks
skoduricg pushed a commit to rentpath/Kingfisher that referenced this pull request Sep 24, 2021
修复各种扩展中`taskIdentifier`的线程安全问题 它在不同线程中调用可能出现崩溃问题
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