Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This fixes a possible race in the migration code #2448

Merged
merged 1 commit into from Jan 6, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 8 additions & 4 deletions hpx/runtime/components/server/migration_support.hpp
Expand Up @@ -72,7 +72,7 @@ namespace hpx { namespace components
}
void unpin()
{
// make sure to always grab to AGAS lock first
// make sure to always grab the AGAS lock first
agas::mark_as_migrated(this->gid_,
[this]() mutable -> std::pair<bool, hpx::future<void> >
{
Expand All @@ -88,8 +88,12 @@ namespace hpx { namespace components
{
was_marked_for_migration_ = false;

hpx::lcos::local::promise<void> p;
std::swap(p, trigger_migration_);

l.unlock();
trigger_migration_.set_value();

p.set_value();
return std::make_pair(true, make_ready_future());
}
}
Expand Down Expand Up @@ -139,9 +143,10 @@ namespace hpx { namespace components

// delay migrate operation until pin count goes to zero
was_marked_for_migration_ = true;
hpx::future<void> f = trigger_migration_.get_future();

l.unlock();
return std::make_pair(true, trigger_migration_.get_future());
return std::make_pair(true, std::move(f));
});
}

Expand All @@ -164,7 +169,6 @@ namespace hpx { namespace components
components::pinned_ptr::create<this_component_type>(lva));
}


// Return whether the given object was migrated, if it was not
// migrated, it also returns a pinned pointer.
static std::pair<bool, components::pinned_ptr>
Expand Down
18 changes: 9 additions & 9 deletions tests/unit/component/migrate_component.cpp
Expand Up @@ -48,7 +48,7 @@ struct test_server
// Components which should be migrated using hpx::migrate<> need to
// be Serializable and CopyConstructable. Components can be
// MoveConstructable in which case the serialized data is moved into the
// components constructor.
// component's constructor.
test_server(test_server const& rhs)
: base_type(rhs), data_(rhs.data_)
{}
Expand Down Expand Up @@ -134,7 +134,7 @@ bool test_migrate_component(hpx::id_type source, hpx::id_type target)
HPX_TEST_EQ(t1.get_data(), 42);

try {
// migrate of t1 to the target
// migrate t1 to the target
test_client t2(hpx::components::migrate(t1, target));

// wait for migration to be done
Expand All @@ -143,7 +143,7 @@ bool test_migrate_component(hpx::id_type source, hpx::id_type target)
// the migrated object should have the same id as before
HPX_TEST_EQ(t1.get_id(), t2.get_id());

// the migrated object should life on the target now
// the migrated object should live on the target now
HPX_TEST_EQ(t2.call(), target);
HPX_TEST_EQ(t2.get_data(), 42);
}
Expand All @@ -170,7 +170,7 @@ bool test_migrate_busy_component(hpx::id_type source, hpx::id_type target)
hpx::future<void> busy_work = t1.busy_work();

try {
// migrate of t1 to the target
// migrate t1 to the target
test_client t2(hpx::components::migrate(t1, target));

HPX_TEST_EQ(t1.get_data(), 42);
Expand All @@ -181,7 +181,7 @@ bool test_migrate_busy_component(hpx::id_type source, hpx::id_type target)
// the migrated object should have the same id as before
HPX_TEST_EQ(t1.get_id(), t2.get_id());

// the migrated object should life on the target now
// the migrated object should live on the target now
HPX_TEST_EQ(t2.call(), target);
HPX_TEST_EQ(t2.get_data(), 42);
}
Expand Down Expand Up @@ -212,7 +212,7 @@ bool test_migrate_component2(hpx::id_type source, hpx::id_type target)
// times
for(std::size_t i = 0; i < N; ++i)
{
// migrate of t1 to the target (loc2)
// migrate t1 to the target (loc2)
test_client t2(hpx::components::migrate(t1, target));

HPX_TEST_EQ(t1.get_data(), 42);
Expand All @@ -223,7 +223,7 @@ bool test_migrate_component2(hpx::id_type source, hpx::id_type target)
// the migrated object should have the same id as before
HPX_TEST_EQ(t1.get_id(), t2.get_id());

// the migrated object should life on the target now
// the migrated object should live on the target now
HPX_TEST_EQ(t2.call(), target);
HPX_TEST_EQ(t2.get_data(), 42);

Expand Down Expand Up @@ -260,7 +260,7 @@ bool test_migrate_busy_component2(hpx::id_type source, hpx::id_type target)
{
for(std::size_t i = 0; i < N; ++i)
{
// migrate of t1 to the target (loc2)
// migrate t1 to the target (loc2)
test_client t2(hpx::components::migrate(t1, target));

HPX_TEST_EQ(t1.get_data(), 42);
Expand All @@ -271,7 +271,7 @@ bool test_migrate_busy_component2(hpx::id_type source, hpx::id_type target)
// the migrated object should have the same id as before
HPX_TEST_EQ(t1.get_id(), t2.get_id());

// the migrated object should life on the target now
// the migrated object should live on the target now
HPX_TEST_EQ(t2.call(), target);
HPX_TEST_EQ(t2.get_data(), 42);

Expand Down