Allow to add interface state delegate in background. #1090
Conversation
|
Thanks, the only other place seems to be remove. The delegate call outs need to be on main. |
Source/ASDisplayNode.mm
Outdated
| for (id <ASInterfaceStateDelegate> delegate in _interfaceStateDelegates) { | ||
| if ([delegate respondsToSelector:@selector(nodeDidLoad)]) { | ||
| [delegate nodeDidLoad]; | ||
| @synchronized (_interfaceStateDelegates) { |
There was a problem hiding this comment.
Please use __instanceLock__.
In some methods, like this one, the lock is acquired earlier (i.e a few lines above). You can take advantage of that by storing the delegates in a local variable while the lock is being held, then use it a bit later on.
There was a problem hiding this comment.
delegates calling like didEnterVisibleState is requring ASAssertUnlocked(instanceLock);
There was a problem hiding this comment.
Ah yeah, that just means that method requires to be called without the lock, but within the method it can acquire the lock itself.
There was a problem hiding this comment.
instanceLock.lock();
NSHashTable * delegates = [_interfaceStateDelegates copy];
instanceLock.unlock();
...
ASAssertUnlocked(instanceLock);
...
for (id delegate in delegates) {
if ([delegate respondsToSelector:@selector(nodeDidLoad)]) {
[delegate nodeDidLoad];
}
}
Will something like this work
|
🚫 CI failed with log |
Source/ASDisplayNode.mm
Outdated
| @@ -460,6 +460,16 @@ - (void)dealloc | |||
|
|
|||
| #pragma mark - Loading | |||
|
|
|||
| #define ASDisplayNodeCallInterfaceStateDelegates(method, __lock) \ | |||
| __lock.lock(); \ | |||
| NSHashTable *delegates = _interfaceStateDelegates; \ | |||
There was a problem hiding this comment.
Don't we need to copy the hash table? Same for a couple of other places.
There was a problem hiding this comment.
Good Point. Thanks.
Source/ASDisplayNode.mm
Outdated
| @@ -460,6 +460,16 @@ - (void)dealloc | |||
|
|
|||
| #pragma mark - Loading | |||
|
|
|||
| #define ASDisplayNodeCallInterfaceStateDelegates(method, __lock) \ | |||
There was a problem hiding this comment.
Since __lock is always the same (i.e __instanceLock__), should we just remove the param entirely?
|
One last thing, please update CHANGELOG. |
* fix SIMULATE_WEB_RESPONSE not imported TextureGroup#449 * Fix to make rangeMode update in right time * remove uncessary assert * Allow to add interface state delegate in background threads. * Allow to add interface state delegate in background threads. * lock around _interfaceStateDelegates * lock _interfaceStateDelegates to local variable * Fix comments * remove extra spaces
No description provided.