Hi,
Some comments inline.
And if that sounds good, I will issue a PR based on this direction, and more
reviews/additions/changes would then be easier. wdyt?
Regards.
---
Yavuz Selim Yilmaz
SUNY at Buffalo
Computer Science and Engineering
PhD Candidate
On Aug 15, 2013, at 1:46 PM, Summers Pittman <supittma(a)redhat.com> wrote:
On 08/15/2013 12:23 PM, Yavuz Selim YILMAZ wrote:
> So, if I get it right, each pipe will have it's runnable list. When the pipe is
cancelled, it'll remove all waiting runnable's from threadpoolexecutor, and cancel
the ones currently running. Am I right?
Not exactly. When the Runnable is finished, it removes itself form the thread pool
executor. You will need some other collection which is modified when a pipe operation
begins or completes.
So, when I cancel currently running runnable's, they'll (automatically) be removed
from the thread pool executor. But, there might be some awaiting runnable's in the
thread pool executor too, or there can't be? I feel like, considering two consecutive
pipe request, one will most likely be waiting in the thread pool executor while the other
is running (or at least there might be such cases depending on the system resources etc.)
>
> If so, then what'd be the proper way of cleaning the successfully completed
runnable's from pipe's list?
For example RestAdapter.java#131:
if (exception == null) {
runnableCollection.remove(this);
callback.onSuccess(this.result);
} else {
runnableCollection.remove(this);
callback.onFailure(exception);
}
Would be my first guess.
In this case, since the runnable will be a static nested class, it'll need a reference
to the outer (i.e. RestAdapter) class. Am I right?
>
> For the loader part, I'll check on how to implement your suggestion,
>
> Regards,
>
> ---
> Yavuz Selim Yilmaz
> SUNY at Buffalo
> Computer Science and Engineering
> PhD Candidate
>
> On Aug 15, 2013, at 10:54 AM, Summers Pittman <supittma(a)redhat.com> wrote:
>
>> Wow you don't make this easy on us ;)
>>
>> Killing the thread/thread pool is probably a bad idea. It may be cleaner to make
the anonymous runner classes in RestAdatper into static classes which implement Runnable
and provide a cancel method. In the RestAdapter you could do some bookkeeping to provide
references to the Threads so you can call cancel on each one which is
currently running.
>>
>> Also, the LoaderAdapters will need a different solution (but theirs is MUCH
easier, you just call cancelLoader and implement onCancel in the Loader implementations).
>>
>> Summers
>>
>> PS bookkeeping is one of my favorite words.
>>
>>
>> On 08/14/2013 03:36 PM, Yavuz Selim YILMAZ wrote:
>>> Hi all,
>>>
>>> I am working on AGDROID-11 [
https://issues.jboss.org/browse/AGDROID-11]. As
suggested on the JIRA, I started with investigating how AeroGear iOS handles pipe cancel
operation.
>>>
>>> -----------------------------------------------------------------------
>>> ---- Skip if you don't want to verify my iOS tracking ----
>>> -----------------------------------------------------------------------
>>> ------------------------------ Start ---------------------------------
>>> -----------------------------------------------------------------------
>>>
>>> First, here is what I could track on the iOS side (please correct me if
there's any mistake):
>>>
>>> - AGRESTPipe cancel method simply calls the cancelAllOperations on its
restClient's operationQueue [1].
>>> - AGRESTPipe's restClient is a AGHttpClient [2].
>>> - AGHttpClient is a AFHttpClient [3].
>>> - AFHttpClient's operationQueue is a NSOperationQueue [4].
>>> - Calling cancelAllOperations on NSOperationQueue sends cancel message to all
Operations in the queue [5]. If the operation is still in the queue (waiting to be
executed), they are not being executed. If the operation is alread started to it's
execution, it's up to the "Operation" how to respond to cancel message.
>>> - Sending cancel message to operations sets their isCancelled flag. Any
custom NSOperation extension is recommended to handle this cancel operation [6].
>>> - AGHttpClient creates the operation by calling
HTTPRequestOperationWithRequest [7], which is AFHttpClient method. It creates either
custom (AFImage, AFJSON, AFPropertyList, AFXML) operation [8] or AFHttpRequestOperation
[9]. All custom operations are AFHttpRequestOperation, which is a AFURLConnectionOperation
[10].
>>> - AFURLConnectionOperation implements cancel method [11] and simply cancels
the operation's NSURLConnection, the connection is cancelled and no delegate is called
[12].
>>> - Then AFURLConnectionOperation calls its delegate method manually to finish
the operation [13].
>>> - The delegate handles the case [14] and finishes the operation [15].
>>>
>>> -----------------------------------------------------------------------
>>> ------------------------------- End ---------------------------------
>>> -----------------------------------------------------------------------
>>> ---- Skip if you don't want to verify my iOS tracking ----
>>> -----------------------------------------------------------------------
>>>
>>>
>>>
>>> Simply, what I understand is, operation itself is responsible to handle the
cancel message sent by any pipe implementation. And on the Android side, the operations
are runnables [16, 17, 18, 19] as far as I could interpret. So, these runnables should be
able to handle cancel messages which means they should catch or throw the thrown
InterruptedException.
>>>
>>> So, the problem is, find a way to send cancel message to the tasks, and
handle the cancel message on the tasks.
>>>
>>>
>>> HOW TO SEND CANCEL MESSAGE TO THE TASKS?
>>>
>>> 1ST PROPOSAL:
>>>
>>> RestAdapter is using ThreadPoolExecutor [20]. So, one way to send cancel
message to all running and queued runnables is to call shutDownNow on the
ThreadPoolExecutor [21]. It has some limitations. First, shut down destroys the
ThreadPoolExecutor, so it needs to be recreated. Second, the current ThredPoolExecutor is
static, means it will cancel all pipes if it is shut down, so it needs to be an instance
variable instead of static.
>>>
>>>
>>> 2ND PROPOSAL:
>>>
>>> Instead of calling THREAD_POOL_EXECUTOR.execute(), if we call
THREAD_POOL_EXECUTOR.submit() [22], then it returns a Future representing the task. Future
has cancel() [23], which prevents running the task if it is not started, and interrupts
the task if it is currently running. Therefore, if we have a collection of <Future>
in RestAdapter (i.e. pipe), then we can cancel the operations by iterating over the
collection and calling cancel() on each Future. In this case, at some point, we need to
clean <Future> collection to get rid of the already cancelled tasks.
>>>
>>>
>>>
>>> HOW TO HANDLE THE CANCEL MESSAGE ON THE TASKS?
>>>
>>> I could only come up with a single solution, which is, adding another catch
block to each of the runnables [16, 17, 18, 19]. Here is what it will look like:
>>>
>>> try {
>>> this.result = ...
>>> } catch (InterruptedException e) {
>>> return; // don't call the callbacks, may need to check the cause of
the interrupt???
>>> } catch (Exception e) {
>>> Log.e(TAG, e.getMessage(), e);
>>> this.exception = e;
>>> }
>>>
>>>
>>>
>>> So, I would like to hear what you think about my solution, and if you have
any corrections, better solutions, comments, anything... All are most welcome.
>>>
>>>
>>> Kind regards,
>>>
>>>
>>>
>>>
>>>
>>> --- REFERENCES ---
>>> [1]
https://github.com/aerogear/aerogear-ios/blob/master/AeroGear-iOS/AeroGea...
>>> [2]
https://github.com/aerogear/aerogear-ios/blob/master/AeroGear-iOS/AeroGea...
>>> [3]
https://github.com/aerogear/aerogear-ios/blob/master/AeroGear-iOS/AeroGea...
>>> [4]
https://github.com/AFNetworking/AFNetworking/blob/master/AFNetworking/AFH...
>>> [5]
http://developer.apple.com/library/ios/DOCUMENTATION/Cocoa/Reference/NSOp...
>>> [6]
http://developer.apple.com/library/mac/documentation/Cocoa/Reference/NSOp...
>>> [7]
https://github.com/aerogear/aerogear-ios/blob/master/AeroGear-iOS/AeroGea...
>>> [8]
https://github.com/AFNetworking/AFNetworking/blob/master/AFNetworking/AFH...
>>> [9]
https://github.com/AFNetworking/AFNetworking/blob/master/AFNetworking/AFH...
>>> [10]
https://github.com/AFNetworking/AFNetworking/blob/master/AFNetworking/AFH...
>>> [11]
https://github.com/AFNetworking/AFNetworking/blob/master/AFNetworking/AFU...
>>> [12]
http://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foun...
>>> [13]
https://github.com/AFNetworking/AFNetworking/blob/master/AFNetworking/AFU...
>>> [14]
https://github.com/AFNetworking/AFNetworking/blob/master/AFNetworking/AFU...
>>> [15]
https://github.com/AFNetworking/AFNetworking/blob/master/AFNetworking/AFU...
>>>
>>> [16]
https://github.com/aerogear/aerogear-android/blob/master/src/org/jboss/ae...
>>> [17]
https://github.com/aerogear/aerogear-android/blob/master/src/org/jboss/ae...
>>> [18]
https://github.com/aerogear/aerogear-android/blob/master/src/org/jboss/ae...
>>> [19]
https://github.com/aerogear/aerogear-android/blob/master/src/org/jboss/ae...
>>> [20]
https://github.com/aerogear/aerogear-android/blob/master/src/org/jboss/ae...
>>> [21]
http://developer.android.com/reference/java/util/concurrent/ThreadPoolExe...
>>> [22]
http://developer.android.com/reference/java/util/concurrent/AbstractExecu...
>>> [23]
http://developer.android.com/reference/java/util/concurrent/Future.html#c...
>>>
>>> ---
>>> Yavuz Selim Yilmaz
>>> SUNY at Buffalo
>>> Computer Science and Engineering
>>> PhD Candidate
>>>
>>>
>>>
>>> _______________________________________________
>>> aerogear-dev mailing list
>>> aerogear-dev(a)lists.jboss.org
>>>
https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>
>> _______________________________________________
>> aerogear-dev mailing list
>> aerogear-dev(a)lists.jboss.org
>>
https://lists.jboss.org/mailman/listinfo/aerogear-dev
>
>
>
> _______________________________________________
> aerogear-dev mailing list
> aerogear-dev(a)lists.jboss.org
>
https://lists.jboss.org/mailman/listinfo/aerogear-dev
_______________________________________________
aerogear-dev mailing list
aerogear-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/aerogear-dev