Re: NSCondition


Sandor Szatmari
 

Dave,

On Mon, Mar 12, 2018 at 1:28 PM, Dave <dave@...> wrote:
Hi Again Sandor,

Yes, I’ve had similar fun doing the same thing here!

Thanks for the heads up on isCancelled and I’ll take your advice and store it as a property.

I’m not sure I understand what you mean with the call to “wait” - please see code revised code below.

See below for what I'm talking about with the -wait call...

Additionally, think about where your testing -isCancelled and breaking out of the loop(s); you have multiple loops.  Does that break, break you out of all loops or just the innermost loop.  Do you want to break out of all loops... if your terminating the thread, probably.  

Where you've placed it, It looks like you can remove an object from the queue and the break the loop without processing it... if the application were not terminating and you were going to resume processing, you would loose that one item...

You also might want to check it at the head of your loop...  and/or check it before you call -wait inside my suggested loop...  In general where and how often you check -isCanceled can determine how responsive and well behaved an app/thread is to terminating...  Just be sure to make sure your leaving everything in an expected/sane state; tearing things down properly... releasing objects... unlocking your conditions... etc.

 

I think I’m already doing what you suggest?

All the Best
Dave

-(void) consumerTask
{
while ([[NSThread currentThread] isCancelled] == NO)
        {
        @autoreleasepool
                {
//**
//**    Wait for a Signal
//**
                [self.pQueueTaskConsumerTaskWakeUpCondition lock];

                   // --------------------------------------------------------------------------------------------
                   //  New Loop
                   // 
                   //  If the NSCondition ever gets signaled, wait again if the queue is empty.
                   //  The documentation for NSCondition explains that the condition when signaled will try to acquire the lock and the proceed
                   //  so once we acquire the lock here the next thing we will do is check the queue.
                   //  If is it not empty, we end the loop and proceed to process.
                   //  If it is empty we call wait again... (This is testing the predicate that has been referred to.)
                   //     The documentation states that calling wait will unlock the condition and and then wait to be signaled...
                   //
                   while ( [self.pQueueTaskProcessingQueue count] == 0 )
                    [self.pQueueTaskConsumerTaskWakeUpCondition wait];
 
//**
//**    Get Head Object - Locked
//**
                self.pQueueTaskLastObjectRemoved = [self.pQueueTaskProcessingQueue queueGetHead];
                self.pQueueTaskProcessingQueueCount = [self.pQueueTaskProcessingQueue queueGetCount];
                [self.pQueueTaskConsumerTaskWakeUpCondition unlock];

//**
//**    Loop to Process All Items
//**
                while (self.pQueueTaskLastObjectRemoved != nil)
                        {
                        if ([[NSThread currentThread] isCancelled] == YES)
                                break;

//**
//**    Process Object - Unlocked
//**
                        [self processObject:self.pQueueTaskLastObjectRemoved];

//**
//**    Get Head Object - Locked
//**
                        [self.pQueueTaskConsumerTaskWakeUpCondition lock];
                        self.pQueueTaskLastObjectRemoved = [self.pQueueTaskProcessingQueue queueGetHead];
                        self.pQueueTaskProcessingQueueCount = [self.pQueueTaskProcessingQueue queueGetCount];
                        [self.pQueueTaskConsumerTaskWakeUpCondition unlock];
                        }
                }
        }
}

> On 12 Mar 2018, at 14:34, Sandor Szatmari <admin.szatmari.net@...> wrote:
>
> Dave,
>
> That's great I'm glad it's working!  I had fun working up the sample... :)  I let it run for a couple of hours watching the processes add a consume data.  It was interesting to see how the random generation and consumption of data fought with each other.
>
> The way I saw it, you conflated the processing of the head object's contents with the processing of the array.  Only the array access (array processing) needs to be protected by the NSCondition...
>
> Before you call it a day consider the following.  I believe you really should have the nested while loop in there around the call to -wait.  My interpretation as to why this is necessary...  I believe it prevents you from acquiring the lock when it's unnecessary to hold the lock; guarding against deadlock conditions and any unnecessary dead time in any thread that may be waiting on the  lock.
>
> The documentation for NSCondition specifically talks about spurious wakes and wanting to guard against this.  Check it out: https://developer.apple.com/documentation/foundation/nscondition.  It has a nice bit of pseudo code illustrating how it should be used.  Reading it, I felt the documentation for this class is very good and worth a read or a reread...
>
> Cheers,
> Sandor
>
>
> On Mon, Mar 12, 2018 at 7:06 AM, Dave <dave@...> wrote:
> Hi,
>
> Thanks for the sample code Sandor, I’ve now got it working correctly now and I’m really pleased with it. I realised what was wrong just before I went to sleep last night and just added the fix and it works! The major difference in the way your project works is that you have a separate Producer Thread, in my case, a delegate method of the producer task is called with the Data on the Main Thread and you must completely deal with the data it passes before the method returns...
>
> The problem was that it was processing the whole queue with the Condition Locked, so nothing else could get it, this is the revised Consumer Task and it works really well:
>
> -(void) consumerTask
> {
> while (YES)
>         {
>         [self.pQueueTaskConsumerTaskWakeUpCondition lock];
>         [self.pQueueTaskConsumerTaskWakeUpCondition wait];
>
> //**
> //**    Get Head Object - Locked
> //**
>         self.pQueueTaskLastObjectRemoved = [self.pQueueTaskProcessingQueue queueGetHead];
>         [self.pQueueTaskConsumerTaskWakeUpCondition unlock];
>
> //**
> //**    Process the Entire Queue
> //**
>         while (self.pQueueTaskLastObjectRemoved != nil)
>                 {
> //**
> //**    Process Object - Unlocked
> //**
>                 [self processObject:self.pQueueTaskLastObjectRemoved];
>
> //**
> //**    Get Head Object - Locked
> //**
>                 [self.pQueueTaskConsumerTaskWakeUpCondition lock];
>                 self.pQueueTaskLastObjectRemoved = [self.pQueueTaskProcessingQueue queueGetHead];
>                 [self.pQueueTaskConsumerTaskWakeUpCondition unlock];
>                 }
>         }
> }
>
> I noticed that your Consumer thread checks “isCancelled”, I’ve added this to my version, although I’m not sure it is needed.
>
> I’ve written this as two generalised classes, a Task Class and a Queue Class, to use it all you need to do is created the Task Object with a Delegate that contains two methods one that is called just before an Object is added to the Queue and one that is called to process the Object. Once I’ve had a chance to tidy it up I’ll send it to anyone that is interested.
>
> Thanks again,
> All the Best
> Dave
>
>
>
>
>
>





Join cocoa@apple-dev.groups.io to automatically receive all group messages.