Ramy Al Zuhouri
Ramy Al Zuhouri

Reputation: 21976

Simple multithreading application sometimes fails

I am doing this multithreading application just to see how the @synchronized directive works.I've read that if all the threads have the same object as argument of @synchronized, then they all wait on the same lock.So I thought to use self as argument, since it's the same for all threads.
In this application there is a text field being edited multiple times by all the thread.I don't care about performance, it's just a test so I don't put the @synchronized directive before the for, but inside it.

The properties that I use:

@property (weak) IBOutlet NSTextField *textField;
@property (nonatomic, copy) NSNumber* value;
@property (nonatomic,copy) NSMutableArray* threads;

The code:

- (void)applicationDidFinishLaunching:(NSNotification *)aNotification
{
    value= @0;
    textField.objectValue= value;
    for(int i=0; i<10; i++)
    {
        NSThread* thread=[[NSThread alloc]initWithTarget: self selector: @selector(routine:) object: nil];
        [threads addObject: thread];
        [thread start];
    }
}

- (void) routine : (id) argument
{
    for(NSUInteger i=0; i<100; i++)
    {
        @synchronized(self)
        {
            value= @(value.intValue+1);
            textField.objectValue= value;
        }
    }
}

Sometimes the application has success and I see 1000 as text field value.But sometimes not, I fear that this is starvation and I don't see anything on the text field, it's empty.I tried the debug but it's hard to see what is wrong, since the criteria of failure seems casual to me, sometimes it just works fine.

SOLUTION

@synthesize threads,value, textField;

- (void)applicationDidFinishLaunching:(NSNotification *)aNotification
{
    value= @0;
    threads=[[NSMutableArray alloc]initWithCapacity: 100];
    for(NSUInteger i=0; i<100; i++)
    {
        NSThread* thread=[[NSThread alloc]initWithTarget: self selector: @selector(routine:) object: nil ];
        [threads addObject: thread];
        [thread start];
    }
}

- (void) routine : (id) arg
{
    for(NSUInteger i=0; i<1000; i++)
    {
        @synchronized(self)
        {
            value= @(value.intValue+1);
            [textField performSelectorOnMainThread: @selector(setObjectValue:) withObject: value waitUntilDone: NO];
        }
    }
}

Upvotes: 2

Views: 235

Answers (2)

Sandeep
Sandeep

Reputation: 21144

You are always updating your UI in background thread. This is not good. You should do it like;

 (void)applicationDidFinishLaunching:(NSNotification *)aNotification
{
    value= @0;
    textField.objectValue= value;
    for(int i=0; i<10; i++)
    {
        NSThread* thread=[[NSThread alloc] initWithTarget: self selector: @selector(routine:) object: nil];
        [threads addObject: thread];
        [thread start];
    }
}

- (void) routine : (id) argument
{
    for(NSUInteger i=0; i<100; i++)
    {
        @synchronized(self)
        {
            value= @(value.intValue+1);
            [textField performSelectorOnMainThread:@selector(setObjectValue:) withObject:value waitUntilDone: NO];    
        }
    }
}

Try locking the instances and use sleep method to make it more smoother.

To lock a variable inside the background thread you first lock it and then set its value and unlock it as;

[NSLock lock];
value=@(value.intValue+1)
[NSLock unlock];

But, you already have @synchronized which makes it safeguard for the variable being accessed from the multiple threads at the same time. I think @synchonized block is easier to understand.

Sleep is more appropriate in this case. If you place a sleep of say 2 seconds inside the thread then, you could see the textField changing every 2 seconds and it is more visible and makes more sense.

[NSThread sleepForTimeInterval:2000] // place this inside you loop after updating the textField and see the effect.

Also it is better to create a runloop and execute the thread in some runloop which is a better practise.

Upvotes: 1

Kurt Revis
Kurt Revis

Reputation: 27994

You are accessing an NSTextField, which is a subclass of NSView, from a non-main thread. That is not a safe thing to do. The results are undefined; sometimes it appears to work, sometimes it doesn't.

Upvotes: 3

Related Questions