Nux

Merge lp:~townsend/nux/fix-animation-crash into lp:nux

  1. fix-animation-crash
  2. Merge into trunk
Proposed by Christopher Townsend
Status: Merged
Approved by: Christopher Townsend
Approved revision: 839
Merged at revision: 838
Proposed branch: lp:~townsend/nux/fix-animation-crash
Merge into: lp:nux
Diff against target: 68 lines (+18/-5)
3 files modified
NuxCore/Animation-inl.h (+7/-2)
NuxCore/Animation.cpp (+5/-1)
tests/gtest-nuxcore-animation.cpp (+6/-2)
To merge this branch: bzr merge lp:~townsend/nux/fix-animation-crash
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Marco Trevisan (Treviño) Approve
Review via email: mp+208218@code.launchpad.net

Commit message

Fix race condition in the Animation destructor which was exposed in a recent Unity change that causes an intermittent crash.

Description of the change

Fix race condition in the Animation destructor which was exposed in a recent Unity change that causes an intermittent crash.

To post a comment you must log in.
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

+1

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:838
http://jenkins.qa.ubuntu.com/job/nux-ci/105/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/nux-trusty-amd64-ci/27/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/nux-trusty-armhf-ci/27/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/nux-trusty-i386-ci/27/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/nux-ci/105/rebuild

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NuxCore/Animation-inl.h'
2--- NuxCore/Animation-inl.h 2012-12-11 22:24:40 +0000
3+++ NuxCore/Animation-inl.h 2014-02-26 14:00:04 +0000
4@@ -134,8 +134,13 @@
5 double value = easing_curve_.ValueForProgress(progress);
6 // These operators work for most if not all the property types we care
7 // about. Should we need more, we'll reevaluate then.
8- current_value_ = start_value_ + ((finish_value_ - start_value_) * value);
9- updated.emit(current_value_);
10+ VALUE_TYPE new_value = start_value_ + ((finish_value_ - start_value_) * value);
11+
12+ if (new_value != current_value_)
13+ {
14+ current_value_ = new_value;
15+ updated.emit(current_value_);
16+ }
17 }
18 }
19
20
21=== modified file 'NuxCore/Animation.cpp'
22--- NuxCore/Animation.cpp 2012-11-05 21:31:06 +0000
23+++ NuxCore/Animation.cpp 2014-02-26 14:00:04 +0000
24@@ -32,7 +32,11 @@
25
26 na::Animation::~Animation()
27 {
28- Stop();
29+ if (state_ != Stopped)
30+ {
31+ if (Controller* controller = Controller::Instance())
32+ controller->RemoveAnimation(this);
33+ }
34 }
35
36 void na::Animation::Pause()
37
38=== modified file 'tests/gtest-nuxcore-animation.cpp'
39--- tests/gtest-nuxcore-animation.cpp 2012-12-12 15:43:04 +0000
40+++ tests/gtest-nuxcore-animation.cpp 2014-02-26 14:00:04 +0000
41@@ -178,7 +178,7 @@
42
43 animation1->Start();
44 animation2->Start();
45- source.tick.emit(10);
46+ source.tick.emit(10000);
47
48 ASSERT_THAT(animation1->CurrentState(), Eq(na::Animation::Running));
49 ASSERT_TRUE(animation2.get() == nullptr);
50@@ -213,13 +213,17 @@
51
52 TEST(TestAnimation, TestDestructorStops)
53 {
54+ na::TickSource source;
55+ MockAnimationController controller(source);
56+
57 nt::TestCallback finished_called;
58 {
59 NiceMock<MockAnimation> animation; // don't care about restart here
60 animation.finished.connect(finished_called.sigc_callback());
61 animation.Start();
62+ ASSERT_TRUE(controller.HasRunningAnimations());
63 }
64- ASSERT_TRUE(finished_called.happened);
65+ ASSERT_FALSE(controller.HasRunningAnimations());
66 }
67
68 TEST(TestAnimation, TestStoppingStoppedDoesntEmitsFinished)

Subscribers

People subscribed via source and target branches