Issue 1218: cancelling ui thread tasks #1395

Artyomcool · 2015-05-04T20:55:25Z

PR implements cancelling by using ids, like in background threads.

…d into TestSampleRoboApplication_

WonderCsabo · 2015-05-04T21:00:03Z

This has merge conflicts. Can you rebase to develop?

Could you squash the changes of the last two commits to the relevant previous commit(s)? These do not add any interesting info later in the history.

Artyomcool · 2015-05-04T21:20:23Z

done

2015-05-05 0:00 GMT+03:00 Csaba Kozák notifications@github.com:

This has merge conflicts. Can you rebase to develop?

Could you squash the changes of the last two commits to the relevant
previous commit(s)? These do not add any interesting info later in the
history.

—
Reply to this email directly or view it on GitHub
#1395 (comment)
.

WonderCsabo · 2015-05-04T22:11:56Z

Thanks!

You have one checkstyle violation. Please edit the commit which added that.

WonderCsabo · 2015-05-04T22:16:04Z

...st-1-5/src/test/java/org/androidannotations/test15/roboguice/TestSampleRoboApplication_.java

There are some unnecessary lines here.

WonderCsabo · 2015-05-05T08:04:22Z

You have 6 Checkstyle violations.

I am sorry, but i just realized what is the clean and Robolectric way of hacking the Handler. You have to create a custom shadow. Create a CustomShadowHandler class and extend from ShadowHandler. Override the needed method. Then create the org.robolectric.Config.properties file in src/test/resources and add this line:

shadows=my.package.CustomShadowHandler

After that you can revert the modifications of the application class. Actually onCreate runs before each test. Also you can get rid of the reflection (i hope). Anyway, it is a more cleaner solution i think.

WonderCsabo · 2015-05-05T08:09:52Z

Also it seem you did not edit the correct commits while rebasing. Just move the test class in 7ff35c9. And change the Handler hack in bd6d00e. Now all these changes reside in 2726211, adding unrelated changes to its diff.

Artyomcool · 2015-05-05T08:39:19Z

Should I restore commit history from the reflog and rebase changes again?
As for shadows... Maybe it is better to migrate to Robolectric 3? Any decision here will be just a dirty workaround, so there is no a good solution. Should we waste our time to implement a better dirty hack?

WonderCsabo · 2015-05-05T08:41:31Z

Should I restore commit history from the reflog and rebase changes again?

If it is easier, yes. :)

Maybe it is better to migrate to Robolectric 3?

The problem is with 3.0 that there is absolutely no ETA on that. We cannot depend on a SNAPSHOT dependency unfortunately.

Thanks!

Artyomcool · 2015-05-05T08:48:31Z

If it is easier, yes. :)

Do you have some easier options? :)

The problem is with 3.0 that there is absolutely no ETA on that. We cannot depend on a SNAPSHOT dependency unfortunately.

I understand, but it wasn't my primary intention. I mean, I think we should wait for Robolectric 3.0 with ANY suitable solution. If you insist, that own shadow is much better and cleaner - ok, will do the refactoring. But I don't think so. In my opinion there is no good solution here, so there is no point to waste time on this.

WonderCsabo · 2015-05-05T08:55:37Z

I think nope. :)

I think the own shadow is cleaner about the implementation. The hack is still there though after all, you are right. Also the current solution gets that field with reflection and sets the new anonymous class. The proposed solution does not have this perf impact. However, it effects all tests instead of just the UiThreadExecutor related ones.

Okay, you can leave it there, but maybe you should call the hack method in prepareTest, i think it makes more sense.

Artyomcool · 2015-05-05T08:56:51Z

Ok, I'll do it in this way.

Artyomcool · 2015-05-05T19:49:05Z

Well, I failed to move commits through history. There are too much conflicts.
If you want to try do it yourself, I can revert branch to state before squashing commits. Should I?

WonderCsabo · 2015-05-05T20:49:41Z

@Artyomcool please do!

Artyomcool · 2015-05-05T21:32:12Z

Done. Now it is right before all rebases, including actualizing develop.

WonderCsabo · 2015-05-05T23:31:43Z

It was a pain to rebase this... It is here. Can you look at the commit diffs to check i did not kill anything accidentally?

Artyomcool · 2015-05-06T07:09:03Z

Good job! You only forgot to move robolectric hack into prepareTest, as you asked. I'm just wondering, how you managed to do this? Looks like you just rewrote every commit by hand.

WonderCsabo · 2015-05-06T07:42:54Z

Thanks. I updated.

I did not rewrite all commits by hand. 😄 Well, i soft reset the last commit, and created two commits of them: adding the hack in the right place, and moving the test to the package. Then i squashed these commits to the right commit in two rebase sessions to keep the number of conflicts as small as possible. Then i rebased the whole thing to develop. You are right, i edited all of the commits not just in the conflicts resolution, because i added formtting and removed Checkstyle errors, etc. But i would not say i rewrote them by hand.

Artyomcool · 2015-05-06T20:13:00Z

Well, where we are?

WonderCsabo · 2015-05-06T20:55:57Z

I think it is ready to be merged.

WonderCsabo · 2015-05-07T12:44:36Z

...ations/androidannotations-api/src/main/java/org/androidannotations/api/UiThreadExecutor.java

@Artyomcool i almost forgot... I told you that we should run this in a try-finally block, so we make sure we remove the token even if the client code throws a RuntimeException.

After you've got an exception here ui thread is effectively dead. Everything after that has no point.

I do not agree. We store everything in static fields, so they are only cleared if the process is killed. If a RuntimeException happen in the ui thread, the Activity will likely crash, but it does not mean the process is killed. I think.

Sorry, but you a not correct here. There is only one ui thread per process. When exception happens during looping, there is no way to restore it, even if you catch it in some way. Also, components never crash by themselves, only with a whole process. And it is very-very good thing, actually.

No need to be sorry. :) You are right. I may have confused this with something
else, and just wrote down without really thinking it though.
I just tested this, and it goes up to ZygoteInit.main and dies.

But we could add some parameter to catch and log exceptions. Not sure if we should.

Nope, that is not our responsibility.

WonderCsabo · 2015-05-07T12:48:46Z

i have one (hopefully last) comment. Can you please reset this branch to mine, then edit the "add: cleaning up the map at cost of synchronization" commit?

WonderCsabo · 2015-05-07T18:11:30Z

Rebased and merged as of a9deed6. Thanks @Artyomcool for your great work on this!

WonderCsabo · 2015-06-08T21:34:57Z

I updated the wiki.

WonderCsabo and others added 10 commits March 28, 2015 11:58

Add id param to @UiThread

1b500aa

Add UiThreadExecutor which handles cancellation

a880692

Handle @UiThread#id

f7ec0c5

Test UiThread task cancellation

a02f0ec

refactoring: reimplemented ui task cancellation in the more straight way

faaadbe

add: lock-free UIThreadHandler implementation

25fa9af

simple and effective implementation of cancelling ui tasks

f0478d7

add: cleaning up the map at cost of synchronization

9aa9580

fix: synchronization for removing tokens

bbe8c02

refactoring: tests moved to test15 package and UI handlers hacks move…

244bb93

…d into TestSampleRoboApplication_

Artyomcool force-pushed the 1218_cancelUiThreadTasks branch 2 times, most recently from 9f75356 to 773a731 Compare May 4, 2015 21:20

WonderCsabo reviewed May 4, 2015
View reviewed changes

Artyomcool force-pushed the 1218_cancelUiThreadTasks branch from 773a731 to 2726211 Compare May 5, 2015 07:28

Artyomcool force-pushed the 1218_cancelUiThreadTasks branch from 2726211 to 244bb93 Compare May 5, 2015 21:31

WonderCsabo reviewed May 7, 2015
View reviewed changes

WonderCsabo closed this May 7, 2015

WonderCsabo added this to the 4.0 milestone May 7, 2015

WonderCsabo added the Merged label Dec 20, 2015

Issue 1218: cancelling ui thread tasks #1395

Issue 1218: cancelling ui thread tasks #1395

Uh oh!

Conversation

Artyomcool commented May 4, 2015

Uh oh!

WonderCsabo commented May 4, 2015

Uh oh!

Artyomcool commented May 4, 2015

Uh oh!

WonderCsabo commented May 4, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WonderCsabo commented May 5, 2015

Uh oh!

WonderCsabo commented May 5, 2015

Uh oh!

Artyomcool commented May 5, 2015

Uh oh!

WonderCsabo commented May 5, 2015

Uh oh!

Artyomcool commented May 5, 2015

Uh oh!

WonderCsabo commented May 5, 2015

Uh oh!

Artyomcool commented May 5, 2015

Uh oh!

Artyomcool commented May 5, 2015

Uh oh!

WonderCsabo commented May 5, 2015

Uh oh!

Artyomcool commented May 5, 2015

Uh oh!

WonderCsabo commented May 5, 2015

Uh oh!

Artyomcool commented May 6, 2015

Uh oh!

WonderCsabo commented May 6, 2015

Uh oh!

Artyomcool commented May 6, 2015

Uh oh!

WonderCsabo commented May 6, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WonderCsabo May 7, 2015 via email

Choose a reason for hiding this comment

Uh oh!

WonderCsabo commented May 7, 2015

Uh oh!

WonderCsabo commented May 7, 2015

Uh oh!

WonderCsabo commented Jun 8, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants