Own implementation of OperationQueue executes before event is added

Question

I know I am going to get the "Should have used OperationQueues" but I had issues with it on the original implementation.. and infact was getting the same error as this. Both have confused me but this one even more so.

I have an add method which is syncronized on the operations and the worker is syncronized on that too.

The issues is that the objects getting picked off (at least to my knowledge) shouldn't be on the list yet. It then calls execute on the same one twice and skips the first one.

So if A B C was added. It might execute A C C and B doesnt even make an appearance. No other class can see operations and the only thing that adds to it is addOperation

Some output from the program.

[Queue Manager] Adding new operation [ 111 ] is <SendOperation: 0x1dd5c510>
[Queue Manager] executing operation [ 112 ]  is <SendOperation: 0x1dd5c510>
[Operation]  executing [ 112 ]
[Queue Manager] Adding new operation [ 112 ] is <SendOperation: 0x1dd266e0>
[Queue Manager] executing operation [ 112 ]  is <SendOperation: 0x1dd266e0>

It gets the address right. But the number wrong. The number is just an int in the Operation object. It is only ever sen on construction. It has not setters, or way of been modified.

What's going on here?

Looks like somehow im duplicating it in to the address of another one?

#import "MyOperationQueue.h"
#import "Operation.h"
@implementation MyOperationQueue

NSMutableArray *operations;
NSCondition *condition;

-(id) init {
    self = [super init];
    if(self != nil) {
        operations = [[NSMutableArray alloc] init];
        condition = [[NSCondition alloc] init];
    }
    return self;
}

- (void) start{
    [self performSelectorInBackground:@selector(run) withObject:self];
}

-(void)run{
    while(YES){

        [condition lock];
        [condition wait];

        NSMutableArray *buffer = [[NSMutableArray alloc] init];
        @synchronized(operations){
            while([operations count] != 0 ) {
                Operation *operation = [operations objectAtIndex:0];
                [operations removeObject:operation];

                NSLog(@"[Queue Manager] executing operation %i is %@",[operation getNumber],operation);
                [operation execute];
            }
        }
        [condition unlock];
    }

}

-(void)addOperation:(id)operation{
    @synchronized(operations){ 
        NSLog(@"[Queue Manager] Adding new operation [ %i ] is %@",[operation getNumber],operation);
        [operations addObject:(operation)];
    }
    [sendCondition signal];
}

@end


@implementation Operation

int idNumber;

-(id) initWithIdNumber:(int)idNumber_{
    self = [super init];
    if( self ) {
        idNumber = idNumber_;
    }
    return self;
}


- (void) execute{
    NSLog(@"[Operation]  executing [ %i ]",idNumber);
}

-(int) getIdNumber{
    return idNumber;
}

@end

Unless your code somehow travels back through time, it's far more likely that you're just accidentally corrupting the transaction ID when you get that sample output. And yeah, you should be using NSOperationQueue for something like this. — Tom Harrington, May 01 '13 at 21:34
replacing the NSOperationQueue was because I thought that I was doing something crazy with it. But infact I've found something stranger happening. I'll add it to the add it to the question. — Andrew, May 01 '13 at 21:56
How would I go about looking for corruption? Is there a better way than just looking anywhere for it and how would I have corrupted it typically. i.e would if we have NSObjects a and b, Then we set a = b, would NSLog(@"%@",a) and NSLog(@"%@",b) print the same address? — Andrew, May 01 '13 at 22:19
Like just looking at my code I only ever init that Operation in once place and it is it is passed to the list via the addOperation method. literally just alloc init and the pass it. — Andrew, May 01 '13 at 22:33
It's hard to say based on the code presented. Your sample output supports the idea that the numbers are corrupt though-- notice that `` is identified both as 111 and 112. — Tom Harrington, May 01 '13 at 22:34
I cant really post too much code. As as you have noticed "SendOperation" has been renamed Operation. Its part of my degree so I cant really upload the project. :( — Andrew, May 01 '13 at 22:41
I think what im going to do is bodge it and copy the data and add the copy. Surely than can't fail. Then I will look in to the real issue after the dissertation is out of the way. :) — Andrew, May 02 '13 at 02:26

score 0 · Answer 1 · answered May 24 '19 at 18:05

Here's the beginning of your Operation implementation:

@implementation Operation

int idNumber;

If you think idNumber is an instance variable, you are mistaken. You have declared a global variable named idNumber. For more details, please read this answer. This mistake is described as “Option 2”.

If you want to declare a (private) instance variable, do it like this:

@implementation Operation {
    int _idNumber; // The _ prefix is a common Objective-C convention.
}

It would be even better to use a property, which you declare in your @interface like this:

@interface Operation: NSObject

@property (nonatomic, readonly) int idNumber;

If you declare it as a readonly property like this,

the compiler generates the instance variable _idNumber for you, so you can remove your instance variable declaration, and
the compiler generates a getter method named idNumber for you, so you can remove your getIdNumber method.

You made the same error with your operations and condition variables in your @implementation MyOperationQueue.

I didn't realize when I posted this that the question was six years old… Well, I hope you finished your dissertation. — rob mayoff, May 25 '19 at 07:03

Own implementation of OperationQueue executes before event is added

1 Answers1