I've been trying to fix this for the past two hours without doing much progress and I figured maybe it was time to try and get some outside help on this one.
Basically, I have three NSOperations (they are actually AFHTTPRequestOperations to be precise, but if I understand correctly there's no issue using them as NSOperations) that I want to run concurrently, and when all of them are complete I want to run a fourth operation that checks for errors during the process and reports them to the user (the NSOperations are of course added inside a NSOperationQueue).
The last operation is a NSBlockOperation that depends on all three of them, and it is correctly invoked only when the three operations are completed. It is structured like this
NSOperation *finishedUploading = [NSBlockOperation blockOperationWithBlock:^{
NSLog(@"Completed. Errors: %@", _errors);
_textArea.editable = YES;
if (![_errors isEqualToString: @""])
{
//Trim the last newline
_errors = [_errors substringToIndex: _errors.length-1];
dispatch_async(dispatch_get_main_queue(), ^{
//Show error message
});
} else {
dispatch_async(dispatch_get_main_queue(), ^{
//Show success dialog
});
}
}];
However, the _errors property I'm trying to access, which is declared as such
@property NSString *errors;
Is incorrectly read in the finishedUploading block. Its value is reported by NSLog as @"", which is the value I set it to before executing the four NSOperations. The property is changed inside each operation's completion block like this
_errors = [_errors stringByAppendingString: @"Error: Test error\n"];
NSLogging the _errors property after assigning it will show the correct value, and even checking the value on subsequent executions of the master method that prepares the NSOperationQueue (before setting it back to @"") reads the correct value, only finishedUploading gets it wrong.
Furthermore, sometimes finishedUploading DOES get the right value, but only after the first NSLog (the subsequent if conditions is evaluated as true).
I assumed it's because the completion block is executed too early and in fact adding a one second delay before executing the last NSOperation does solve the issue, but it's not an optimal solution as it adds a useless delay (half a second does not even work). I have tried looking around for solutions but even using @synchronized(_errors) and adding (retain) to _errors doesn't help.
Apple's documentation insists that NSString is thread safe so I'm not sure what I'm doing wrong, I assumed appending to the string somehow caused this but even setting the string directly causes the issue.
EDIT
I have changed the assignment to _errors = @"Error: Test error\n";
just in case appending the string was causing the issues
I have also noticed the NSLogs are arriving out of order (finishedUploading's NSLog comes before the NSLog inside the block that changes _errors's value)
Block literals make const copies of captured local state. The _errors
variable is a local reference -- it's not the same thing as the _errors
instance variable in the enclosing scope. The value of the _errors
reference within the block is initialized when the block is defined, not when the block is executed. That's why you're not seeing an updated value.
In general, referencing an instance variable from within a block tends to be confusing. And if it worked the way you were expecting, that would conceptually represent a significant violation of encapsulation, so perhaps it's best that it doesn't.
In any case, consider using accessor methods from within a block to access the state of an object in the enclosing scope, instead of taking a snapshot of the object's instance variable(s) by referencing them directly. More generally, when using a declared a property, prefer using the property's accessor's to directly getting and setting the underlying instance variable; that can help you avoid little pitfalls like this one.
Your rewritten block might look something like the following:
NSOperation *finishedUploading = [NSBlockOperation blockOperationWithBlock:^{
NSLog(@"Completed. Errors: %@", self.errors);
self.textArea.editable = YES;
if (![self.errors isEqualToString:@""])
{
//Trim the last newline
self.errors = [self.errors substringToIndex:self.errors.length-1];
dispatch_async(dispatch_get_main_queue(), ^{
//Show error message
});
} else {
dispatch_async(dispatch_get_main_queue(), ^{
//Show success dialog
});
}
}];
Note that the block is capturing self
(which was also true when it directly accessed the ivar), so it's possible that you're creating a retain cycle. To avoid that, see SO questions on breaking block retain cycles, for example this one: Weak references in blocks and retain cycles