Skip to content

Commit eb715c7

Browse files
paulb777maksymmalyhin
paulb777
authored and
maksymmalyhin
committed
More safely manage background tasks (#3633) (#3635)
* More safely manage background tasks Delete a test that can't be run now. * Move state saving back to being async.
1 parent 51fea2a commit eb715c7

File tree

6 files changed

+53
-42
lines changed

6 files changed

+53
-42
lines changed

‎GoogleDataTransport/GDTLibrary/GDTPlatform.m‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,9 @@ - (GDTBackgroundIdentifier)beginBackgroundTaskWithExpirationHandler:(void (^)(vo
105105
}
106106

107107
- (void)endBackgroundTask:(GDTBackgroundIdentifier)bgID {
108-
[[self sharedApplicationForBackgroundTask] endBackgroundTask:bgID];
108+
if (bgID != GDTBackgroundIdentifierInvalid) {
109+
[[self sharedApplicationForBackgroundTask] endBackgroundTask:bgID];
110+
}
109111
}
110112

111113
#pragma mark - App environment helpers

‎GoogleDataTransport/GDTLibrary/GDTStorage.m‎

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,12 @@ - (instancetype)init {
7676
- (void)storeEvent:(GDTEvent *)event {
7777
[self createEventDirectoryIfNotExists];
7878

79-
if (_backgroundID == GDTBackgroundIdentifierInvalid) {
80-
_backgroundID = [[GDTApplication sharedApplication] beginBackgroundTaskWithExpirationHandler:^{
81-
if (self->_backgroundID != GDTBackgroundIdentifierInvalid) {
82-
[[GDTApplication sharedApplication] endBackgroundTask:self->_backgroundID];
83-
self->_backgroundID = GDTBackgroundIdentifierInvalid;
79+
__block GDTBackgroundIdentifier bgID = GDTBackgroundIdentifierInvalid;
80+
if (_runningInBackground) {
81+
bgID = [[GDTApplication sharedApplication] beginBackgroundTaskWithExpirationHandler:^{
82+
if (bgID != GDTBackgroundIdentifierInvalid) {
83+
[[GDTApplication sharedApplication] endBackgroundTask:bgID];
84+
bgID = GDTBackgroundIdentifierInvalid;
8485
}
8586
}];
8687
}
@@ -111,8 +112,8 @@ - (void)storeEvent:(GDTEvent *)event {
111112
[self.uploadCoordinator forceUploadForTarget:target];
112113
}
113114

114-
// If running in the background, save state to disk and end the associated background task.
115-
if (self->_backgroundID != GDTBackgroundIdentifierInvalid) {
115+
// Write state to disk.
116+
if (self->_runningInBackground) {
116117
if (@available(macOS 10.13, iOS 11.0, tvOS 11.0, *)) {
117118
NSData *data = [NSKeyedArchiver archivedDataWithRootObject:self
118119
requiringSecureCoding:YES
@@ -123,8 +124,12 @@ - (void)storeEvent:(GDTEvent *)event {
123124
[NSKeyedArchiver archiveRootObject:self toFile:[GDTStorage archivePath]];
124125
#endif
125126
}
126-
[[GDTApplication sharedApplication] endBackgroundTask:self->_backgroundID];
127-
self->_backgroundID = GDTBackgroundIdentifierInvalid;
127+
}
128+
129+
// If running in the background, save state to disk and end the associated background task.
130+
if (bgID != GDTBackgroundIdentifierInvalid) {
131+
[[GDTApplication sharedApplication] endBackgroundTask:bgID];
132+
bgID = GDTBackgroundIdentifierInvalid;
128133
}
129134
});
130135
}
@@ -216,14 +221,7 @@ - (void)appWillForeground:(GDTApplication *)app {
216221
}
217222

218223
- (void)appWillBackground:(GDTApplication *)app {
219-
if (_backgroundID == GDTBackgroundIdentifierInvalid) {
220-
_backgroundID = [[GDTApplication sharedApplication] beginBackgroundTaskWithExpirationHandler:^{
221-
if (self->_backgroundID != GDTBackgroundIdentifierInvalid) {
222-
[[GDTApplication sharedApplication] endBackgroundTask:self->_backgroundID];
223-
self->_backgroundID = GDTBackgroundIdentifierInvalid;
224-
}
225-
}];
226-
}
224+
self->_runningInBackground = YES;
227225
dispatch_async(_storageQueue, ^{
228226
if (@available(macOS 10.13, iOS 11.0, tvOS 11.0, *)) {
229227
NSData *data = [NSKeyedArchiver archivedDataWithRootObject:self
@@ -236,10 +234,18 @@ - (void)appWillBackground:(GDTApplication *)app {
236234
#endif
237235
}
238236
});
237+
238+
// Create an immediate background task to run until the end of the current queue of work.
239+
__block GDTBackgroundIdentifier bgID = [app beginBackgroundTaskWithExpirationHandler:^{
240+
if (bgID != GDTBackgroundIdentifierInvalid) {
241+
[app endBackgroundTask:bgID];
242+
bgID = GDTBackgroundIdentifierInvalid;
243+
}
244+
}];
239245
dispatch_async(_storageQueue, ^{
240-
if (self->_backgroundID != GDTBackgroundIdentifierInvalid) {
241-
[app endBackgroundTask:self->_backgroundID];
242-
self->_backgroundID = GDTBackgroundIdentifierInvalid;
246+
if (bgID != GDTBackgroundIdentifierInvalid) {
247+
[app endBackgroundTask:bgID];
248+
bgID = GDTBackgroundIdentifierInvalid;
243249
}
244250
});
245251
}

‎GoogleDataTransport/GDTLibrary/GDTTransformer.m‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,10 @@ - (void)transformEvent:(GDTEvent *)event
5151
__block GDTBackgroundIdentifier bgID = GDTBackgroundIdentifierInvalid;
5252
if (_runningInBackground) {
5353
bgID = [[GDTApplication sharedApplication] beginBackgroundTaskWithExpirationHandler:^{
54-
[[GDTApplication sharedApplication] endBackgroundTask:bgID];
54+
if (bgID != GDTBackgroundIdentifierInvalid) {
55+
[[GDTApplication sharedApplication] endBackgroundTask:bgID];
56+
bgID = GDTBackgroundIdentifierInvalid;
57+
}
5558
}];
5659
}
5760
dispatch_async(_eventWritingQueue, ^{
@@ -71,6 +74,7 @@ - (void)transformEvent:(GDTEvent *)event
7174
[self.storageInstance storeEvent:transformedEvent];
7275
if (self->_runningInBackground) {
7376
[[GDTApplication sharedApplication] endBackgroundTask:bgID];
77+
bgID = GDTBackgroundIdentifierInvalid;
7478
}
7579
});
7680
}
@@ -86,10 +90,16 @@ - (void)appWillForeground:(GDTApplication *)app {
8690
- (void)appWillBackground:(GDTApplication *)app {
8791
// Create an immediate background task to run until the end of the current queue of work.
8892
__block GDTBackgroundIdentifier bgID = [app beginBackgroundTaskWithExpirationHandler:^{
89-
[app endBackgroundTask:bgID];
93+
if (bgID != GDTBackgroundIdentifierInvalid) {
94+
[app endBackgroundTask:bgID];
95+
bgID = GDTBackgroundIdentifierInvalid;
96+
}
9097
}];
9198
dispatch_async(_eventWritingQueue, ^{
92-
[app endBackgroundTask:bgID];
99+
if (bgID != GDTBackgroundIdentifierInvalid) {
100+
[app endBackgroundTask:bgID];
101+
bgID = GDTBackgroundIdentifierInvalid;
102+
}
93103
});
94104
}
95105

‎GoogleDataTransport/GDTLibrary/GDTUploadCoordinator.m‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,10 +192,16 @@ - (void)appWillBackground:(GDTApplication *)app {
192192

193193
// Create an immediate background task to run until the end of the current queue of work.
194194
__block GDTBackgroundIdentifier bgID = [app beginBackgroundTaskWithExpirationHandler:^{
195-
[app endBackgroundTask:bgID];
195+
if (bgID != GDTBackgroundIdentifierInvalid) {
196+
[app endBackgroundTask:bgID];
197+
bgID = GDTBackgroundIdentifierInvalid;
198+
}
196199
}];
197200
dispatch_async(_coordinationQueue, ^{
198-
[app endBackgroundTask:bgID];
201+
if (bgID != GDTBackgroundIdentifierInvalid) {
202+
[app endBackgroundTask:bgID];
203+
bgID = GDTBackgroundIdentifierInvalid;
204+
}
199205
});
200206
}
201207

‎GoogleDataTransport/GDTLibrary/Private/GDTStorage_Private.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,9 @@ NS_ASSUME_NONNULL_BEGIN
3535
/** The upload coordinator instance used by this storage instance. */
3636
@property(nonatomic) GDTUploadCoordinator *uploadCoordinator;
3737

38-
/** The background identifier, not invalid if running in the background. */
39-
@property(nonatomic) GDTBackgroundIdentifier backgroundID;
38+
/** If YES, every call to -storeLog results in background task and serializes the singleton to disk.
39+
*/
40+
@property(nonatomic) BOOL runningInBackground;
4041

4142
/** Returns the path to the keyed archive of the singleton. This is where the singleton is saved
4243
* to disk during certain app lifecycle events.

‎GoogleDataTransport/GDTTests/Unit/GDTStorageTest.m‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -360,18 +360,4 @@ - (void)testQoSTierFast {
360360
});
361361
}
362362

363-
/** Tests a deadlock condition in which a background signal is sent during -storeEvent:. */
364-
- (void)testStoreEventDeadlockFromBackgrounding {
365-
[GDTStorage sharedInstance].backgroundID = 1234;
366-
for (int i = 0; i < 10000; i++) {
367-
GDTEvent *event = [[GDTEvent alloc] initWithMappingID:@"404" target:kGDTTargetCCT];
368-
event.dataObjectTransportBytes = [@"testString" dataUsingEncoding:NSUTF8StringEncoding];
369-
event.qosTier = GDTEventQoSFast;
370-
event.clockSnapshot = [GDTClock snapshot];
371-
[[GDTStorage sharedInstance] storeEvent:event];
372-
}
373-
dispatch_sync(dispatch_get_global_queue(QOS_CLASS_USER_INTERACTIVE, 0), ^{
374-
});
375-
}
376-
377363
@end

0 commit comments

Comments
 (0)