react-native/RNTester/RNTesterUnitTests/RCTEventDispatcherTests.m

262 lines
7.7 KiB
Mathematica
Raw Normal View History

/**
* Copyright (c) 2015-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
*/
#import <UIKit/UIKit.h>
#import <XCTest/XCTest.h>
#import <OCMock/OCMock.h>
#import <React/RCTEventDispatcher.h>
@interface RCTTestEvent : NSObject <RCTEvent>
@property (atomic, assign, readwrite) BOOL canCoalesce;
@end
@implementation RCTTestEvent
{
NSDictionary<NSString *, id> *_body;
}
@synthesize viewTag = _viewTag;
@synthesize eventName = _eventName;
@synthesize coalescingKey = _coalescingKey;
- (instancetype)initWithViewTag:(NSNumber *)viewTag
eventName:(NSString *)eventName
body:(NSDictionary<NSString *, id> *)body
coalescingKey:(uint16_t)coalescingKey
{
if (self = [super init]) {
_viewTag = viewTag;
_eventName = eventName;
_body = body;
_canCoalesce = YES;
_coalescingKey = coalescingKey;
}
return self;
}
- (id<RCTEvent>)coalesceWithEvent:(id<RCTEvent>)newEvent
{
return newEvent;
}
+ (NSString *)moduleDotMethod
{
return @"MyCustomEventemitter.emit";
}
- (NSArray *)arguments
{
return @[_eventName, _body];
}
@end
@interface RCTDummyBridge : RCTBridge
- (void)dispatchBlock:(dispatch_block_t)block
queue:(dispatch_queue_t)queue;
@end
@implementation RCTDummyBridge
- (void)dispatchBlock:(dispatch_block_t)block
queue:(dispatch_queue_t)queue
{}
@end
@interface RCTEventDispatcherTests : XCTestCase
@end
@implementation RCTEventDispatcherTests
{
id _bridge;
RCTEventDispatcher *_eventDispatcher;
NSString *_eventName;
NSDictionary<NSString *, id> *_body;
RCTTestEvent *_testEvent;
NSString *_JSMethod;
}
- (void)setUp
{
[super setUp];
_bridge = [OCMockObject mockForClass:[RCTDummyBridge class]];
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
_eventDispatcher = [RCTEventDispatcher new];
Refactored module access to allow for lazy loading Summary: public The `bridge.modules` dictionary provides access to all native modules, but this API requires that every module is initialized in advance so that any module can be accessed. This diff introduces a better API that will allow modules to be initialized lazily as they are needed, and deprecates `bridge.modules` (modules that use it will still work, but should be rewritten to use `bridge.moduleClasses` or `-[bridge moduleForName/Class:` instead. The rules are now as follows: * Any module that overrides `init` or `setBridge:` will be initialized on the main thread when the bridge is created * Any module that implements `constantsToExport:` will be initialized later when the config is exported (the module itself will be initialized on a background queue, but `constantsToExport:` will still be called on the main thread. * All other modules will be initialized lazily when a method is first called on them. These rules may seem slightly arcane, but they have the advantage of not violating any assumptions that may have been made by existing code - any module written under the original assumption that it would be initialized synchronously on the main thread when the bridge is created should still function exactly the same, but modules that avoid overriding `init` or `setBridge:` will now be loaded lazily. I've rewritten most of the standard modules to take advantage of this new lazy loading, with the following results: Out of the 65 modules included in UIExplorer: * 16 are initialized on the main thread when the bridge is created * A further 8 are initialized when the config is exported to JS * The remaining 41 will be initialized lazily on-demand Reviewed By: jspahrsummers Differential Revision: D2677695 fb-gh-sync-id: 507ae7e9fd6b563e89292c7371767c978e928f33
2015-11-25 11:09:00 +00:00
[_eventDispatcher setValue:_bridge forKey:@"bridge"];
_eventName = RCTNormalizeInputEventName(@"sampleEvent");
_body = @{ @"foo": @"bar" };
_testEvent = [[RCTTestEvent alloc] initWithViewTag:nil
eventName:_eventName
body:_body
coalescingKey:0];
_JSMethod = [[_testEvent class] moduleDotMethod];
}
- (void)testLegacyEventsAreImmediatelyDispatched
{
[[_bridge expect] enqueueJSCall:@"RCTDeviceEventEmitter"
method:@"emit"
args:[_testEvent arguments]
completion:NULL];
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
[_eventDispatcher sendDeviceEventWithName:_eventName body:_body];
#pragma clang diagnostic pop
[_bridge verify];
}
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
- (void)testNonCoalescingEventIsImmediatelyDispatched
{
_testEvent.canCoalesce = NO;
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
[[_bridge expect] dispatchBlock:OCMOCK_ANY queue:RCTJSThread];
[_eventDispatcher sendEvent:_testEvent];
[_bridge verify];
}
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
- (void)testCoalescingEventIsImmediatelyDispatched
{
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
_testEvent.canCoalesce = YES;
[[_bridge expect] dispatchBlock:OCMOCK_ANY queue:RCTJSThread];
[_eventDispatcher sendEvent:_testEvent];
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
[_bridge verify];
}
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
- (void)testMultipleEventsResultInOnlyOneDispatchAfterTheFirstOne
{
[[_bridge expect] dispatchBlock:OCMOCK_ANY queue:RCTJSThread];
[_eventDispatcher sendEvent:_testEvent];
[_eventDispatcher sendEvent:_testEvent];
[_eventDispatcher sendEvent:_testEvent];
[_eventDispatcher sendEvent:_testEvent];
[_eventDispatcher sendEvent:_testEvent];
[_bridge verify];
}
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
- (void)testRunningTheDispatchedBlockResultInANewOneBeingEnqueued
{
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
__block dispatch_block_t eventsEmittingBlock;
[[_bridge expect] dispatchBlock:[OCMArg checkWithBlock:^(dispatch_block_t block) {
eventsEmittingBlock = block;
return YES;
}] queue:RCTJSThread];
[_eventDispatcher sendEvent:_testEvent];
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
[_bridge verify];
// eventsEmittingBlock would be called when js is no longer busy, which will result in emitting events
[[_bridge expect] enqueueJSCall:[[_testEvent class] moduleDotMethod]
args:[_testEvent arguments]];
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
eventsEmittingBlock();
[_bridge verify];
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
[[_bridge expect] dispatchBlock:OCMOCK_ANY queue:RCTJSThread];
[_eventDispatcher sendEvent:_testEvent];
[_bridge verify];
}
- (void)testBasicCoalescingReturnsLastEvent
{
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
__block dispatch_block_t eventsEmittingBlock;
[[_bridge expect] dispatchBlock:[OCMArg checkWithBlock:^(dispatch_block_t block) {
eventsEmittingBlock = block;
return YES;
}] queue:RCTJSThread];
[[_bridge expect] enqueueJSCall:[[_testEvent class] moduleDotMethod]
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
args:[_testEvent arguments]];
RCTTestEvent *ignoredEvent = [[RCTTestEvent alloc] initWithViewTag:nil
eventName:_eventName
body:@{ @"other": @"body" }
coalescingKey:0];
[_eventDispatcher sendEvent:ignoredEvent];
[_eventDispatcher sendEvent:_testEvent];
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
eventsEmittingBlock();
[_bridge verify];
}
- (void)testDifferentEventTypesDontCoalesce
{
NSString *firstEventName = RCTNormalizeInputEventName(@"firstEvent");
RCTTestEvent *firstEvent = [[RCTTestEvent alloc] initWithViewTag:nil
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
eventName:firstEventName
body:_body
coalescingKey:0];
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
__block dispatch_block_t eventsEmittingBlock;
[[_bridge expect] dispatchBlock:[OCMArg checkWithBlock:^(dispatch_block_t block) {
eventsEmittingBlock = block;
return YES;
}] queue:RCTJSThread];
[[_bridge expect] enqueueJSCall:[[_testEvent class] moduleDotMethod]
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
args:[firstEvent arguments]];
[[_bridge expect] enqueueJSCall:[[_testEvent class] moduleDotMethod]
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
args:[_testEvent arguments]];
[_eventDispatcher sendEvent:firstEvent];
[_eventDispatcher sendEvent:_testEvent];
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
eventsEmittingBlock();
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
[_bridge verify];
}
- (void)testSameEventTypesWithDifferentCoalesceKeysDontCoalesce
{
NSString *eventName = RCTNormalizeInputEventName(@"firstEvent");
RCTTestEvent *firstEvent = [[RCTTestEvent alloc] initWithViewTag:nil
eventName:eventName
body:_body
coalescingKey:0];
RCTTestEvent *secondEvent = [[RCTTestEvent alloc] initWithViewTag:nil
eventName:eventName
body:_body
coalescingKey:1];
__block dispatch_block_t eventsEmittingBlock;
[[_bridge expect] dispatchBlock:[OCMArg checkWithBlock:^(dispatch_block_t block) {
eventsEmittingBlock = block;
return YES;
}] queue:RCTJSThread];
[[_bridge expect] enqueueJSCall:[[_testEvent class] moduleDotMethod]
args:[firstEvent arguments]];
[[_bridge expect] enqueueJSCall:[[_testEvent class] moduleDotMethod]
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
args:[secondEvent arguments]];
[_eventDispatcher sendEvent:firstEvent];
[_eventDispatcher sendEvent:secondEvent];
[_eventDispatcher sendEvent:firstEvent];
[_eventDispatcher sendEvent:secondEvent];
[_eventDispatcher sendEvent:secondEvent];
[_eventDispatcher sendEvent:firstEvent];
[_eventDispatcher sendEvent:firstEvent];
better event emitting II: no deadlocks Summary:D3092867 / 1d3db4c5dc8763d16f2d051fdf04a2976c0fb154 caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc3066144a48fc13bb7832abc9e645024fb88. The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another, since a held lock was being accessed the dispatched block and was released after the dispatch. Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread. This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase. A fix for this is pretty simple. We will release the lock before dispatching the block. However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed. My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT. (I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.) The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone). But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread. Therefore nothing bad can happen. So this diff brings back the reverted diff and adds to it the fix for the deadlock. Reviewed By: javache Differential Revision: D3130375 fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11 fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-03 06:30:26 +00:00
eventsEmittingBlock();
[_bridge verify];
}
@end