Concurrency issue for cancellation of long-poll loop

Question

I have an issue which I hope I will solve by writing this question but if not I will post and see if anyone can help.

I am using a client library (which is poorly written I feel) to interact with a real time chat server that utilises COMET style long-polling over HTTP. I'm having issues with cancelling the long-poll in certain situations and suspect I may need to add some concurrency handling code but am finding it hard to work out the best way to do this for the below reasons.

The subscribing code (that inits the long poll) is implemented as a big loop with the following

doLongPoll()
{
    while(true)
    }
        //IF channel field boolean unsubscribe == TRUE, if so BREAK;
        //perform GET request (and store channel HTTPClient used for this call)
        //remove HTTPClient used for this call
        //IF channel field boolean unsubscribe == true, if so BREAK;
        //IF connection problem sleep(1500) then CONTINUE
        //post received data to listeners
    }
}

The unsubscribe call (that will called on another thread)

unsubscribe()
{
    //set channel field boolean unsubscribe == FALSE
    //get channel HTTPClient and shutdown
}

I have isolated the problem case where the operations are interleaving. To me this seems a result of the code being multi-threaded and the client code not being thread safe. Also poor management and non-reuse of the httpClient is not helping.

One of the issues I'm having is below where the unsubscribe call does not stop the next getRequest from taking place.

THREAD 1 (polling)                      THREAD 2
--------                                --------
do unsubscribe check (pass)
                                        unsubscribe called
                                        set unsubscribe = true
                                        check if httpClient saved (none)
perform getRequest (save HttpClient first)

I would like to know what people think the best approach to this problem would be (time is also limited so I cant rewrite too much of the code!)

To fix this i would thought i could use a syncronisation block from the first unsubscribe check on thread 1 up to the point the httpClient is saved just before the actual get request is performed and syncronise the unsubscribe method using the same lock. This is not practical at the moment as the first sync block mentioned would start in one method call and finish further down the method call chain (due to how the lib is written) - which feels very wrong so some refactoring would be needed.

OR I could just create a single httpClient per channel rather than per request and then it could always be shutdown and I could potentially ignore the syncronisation (i think).

OR as suggested below I could use interrupts for the same purpose

Any suggestions would be welcome - I will edit if I have any progress!

Thanks

Answer 1

I bought Java Concurrency in Practice as a result of this problem and found they discuss an issue very much like this one in Chapter 7: Cancellation and shutdown which can be summarised by the quote

Interruption is usually the most successful way to implement cancellation

As the HttpClient is blocking socket IO that does not support interruption so I have a two pronged approach. I allocate my httpClient, I check for interruption just before the actual httpClient.execute() call and in my unsubscribe() method interrupt the thread and then call httpClient.getConnectionManager().shutdown();. This seems to take care of my problem and was a very simple change. No more interleaving issues!

I also set the boolean unsubscribe field to volatile as suggested which I shouldve done before - this alone however would not have solved the issue

Answer 2

1) Set unsubscribe as volatile

2) For greater assurance, protect access (read/write) to unsubscribe with, for example, a Semaphore