Skip to content
This repository has been archived by the owner on Sep 4, 2021. It is now read-only.

Commit

Permalink
[subscriber] consistent data update sequence
Browse files Browse the repository at this point in the history
updateFromRemoteCache() always get most recent data, so no need in data event
also introducing race condition.

Signed-off-by: Denis Zalevskiy <denis.zalevskiy@jolla.com>
  • Loading branch information
Denis Zalevskiy committed Nov 12, 2015
1 parent 4cf9521 commit 786dda7
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 24 deletions.
31 changes: 8 additions & 23 deletions src/contextkit-subscriber/property.cpp
Expand Up @@ -132,7 +132,6 @@ class Event : public QEvent
Subscribed,
Write,
Refresh,
Data,
WriteStatus,
Ready
};
Expand Down Expand Up @@ -312,17 +311,6 @@ class DataReadyEvent : public ReplyEvent
{}
};

class DataReplyEvent : public ReplyEvent
{
public:
DataReplyEvent(QVariant v, target_handle const &tgt)
: ReplyEvent(Event::Data, tgt)
, data_(std::move(v))
{}

QVariant data_;
};

class SubscribeRequest : public Event
{
public:
Expand All @@ -348,7 +336,7 @@ class SubscribeRequest : public Event
SubscribeRequest::~SubscribeRequest()
{
auto notify_fn = [this]() {
tgt_->postEvent(new DataReplyEvent(result, tgt_));
tgt_->dataReady(tgt_);
value_.set_value(result);
};
execute_nothrow(notify_fn, __PRETTY_FUNCTION__);
Expand Down Expand Up @@ -820,11 +808,14 @@ PropertyMonitor::monitor_ptr ContextPropertyPrivate::actor()

void ContextPropertyPrivate::onChanged(QVariant v) const
{
if (state_ == Subscribing)
bool subscribing = state_ == Subscribing;
if (subscribing)
state_ = Subscribed;

if (update(v))
if (update(v) || subscribing) {
debug::debug("Notify data ready", key_, v);
emit valueChanged();
}
}

void ContextPropertyPrivate::postEvent(ReplyEvent *e)
Expand All @@ -836,7 +827,6 @@ void ContextPropertyPrivate::postEvent(ReplyEvent *e)
bool ContextPropertyPrivate::event(QEvent *e)
{
using statefs::qt::Event;
using statefs::qt::DataReplyEvent;
using statefs::qt::DataReadyEvent;
if (e->type() < QEvent::User)
return QObject::event(e);
Expand All @@ -845,11 +835,6 @@ bool ContextPropertyPrivate::event(QEvent *e)
auto fn = [this, e, &res]() {
auto t = static_cast<Event::Type>(e->type());
switch (t) {
case Event::Data: {
auto p = EVENT_CAST(e, DataReplyEvent);
if (p) onChanged(std::move(p->data_));
break;
}
case Event::Ready: {
auto p = EVENT_CAST(e, DataReadyEvent);
if (p) updateFromRemoteCache(p);
Expand Down Expand Up @@ -1003,9 +988,9 @@ void ContextPropertyPrivate::updateFromRemoteCache(statefs::qt::DataReadyEvent *
// called from the object thread
update_queued_.clear(std::memory_order_release);
// cache is attached from an other thread, so save pointer copy
auto pcache = remote_cache_;
auto pcache = remote_cache_.lock();
if (pcache)
onChanged(remote_cache_->load());
onChanged(pcache->load());
}


Expand Down
3 changes: 2 additions & 1 deletion src/contextkit-subscriber/property.hpp
Expand Up @@ -269,11 +269,12 @@ class ContextPropertyPrivate : public QObject

// TODO move functionality to the separate interface
friend class statefs::qt::Property;
friend class statefs::qt::SubscribeRequest;
void attachCache(std::shared_ptr<statefs::qt::Cache>) const;
void dataReady(statefs::qt::target_handle);
void updateFromRemoteCache(statefs::qt::DataReadyEvent *);

mutable std::shared_ptr<statefs::qt::Cache> remote_cache_;
mutable std::weak_ptr<statefs::qt::Cache> remote_cache_;
mutable std::atomic_flag update_queued_;
};

Expand Down

0 comments on commit 786dda7

Please sign in to comment.