Fix races in get_group_force.

GitOrigin-RevId: 30b0902bd5ebaee142f45e06d1d7be6cf6d18762
This commit is contained in:
levlam 2018-12-02 22:04:47 +03:00
parent 3e843ece57
commit 013afad0ca
4 changed files with 27 additions and 10 deletions

View File

@ -8280,6 +8280,11 @@ void MessagesManager::set_dialog_last_read_inbox_message_id(Dialog *d, MessageId
if (total_count == 0) { if (total_count == 0) {
set_dialog_last_notification(d, 0, NotificationId(), "set_dialog_last_read_inbox_message_id"); set_dialog_last_notification(d, 0, NotificationId(), "set_dialog_last_read_inbox_message_id");
} }
total_count -= static_cast<int32>(d->pending_new_message_notifications.size());
if (total_count < 0) {
LOG(ERROR) << "Total notification count is negative in " << d->dialog_id;
total_count = 0;
}
send_closure_later(G()->notification_manager(), &NotificationManager::remove_notification_group, send_closure_later(G()->notification_manager(), &NotificationManager::remove_notification_group,
d->message_notification_group_id, NotificationId(), d->last_read_inbox_message_id, total_count, d->message_notification_group_id, NotificationId(), d->last_read_inbox_message_id, total_count,
Promise<Unit>()); Promise<Unit>());
@ -17612,9 +17617,13 @@ MessagesManager::MessageNotificationGroup MessagesManager::get_message_notificat
VLOG(notifications) << "Found " << d->dialog_id << " by " << group_id; VLOG(notifications) << "Found " << d->dialog_id << " by " << group_id;
result.dialog_id = d->dialog_id; result.dialog_id = d->dialog_id;
result.total_count = get_dialog_pending_notification_count(d); result.total_count = get_dialog_pending_notification_count(d);
result.total_count -= static_cast<int32>(d->pending_new_message_notifications.size());
if (result.total_count < 0) {
LOG(ERROR) << "Total notification count is negative in " << d->dialog_id;
result.total_count = 0;
}
result.notifications = get_message_notifications_from_database( result.notifications = get_message_notifications_from_database(
d, d->first_new_notification_id.is_valid() ? d->first_new_notification_id : NotificationId::max(), d, NotificationId::max(), td_->notification_manager_->get_max_notification_group_size());
td_->notification_manager_->get_max_notification_group_size());
int32 last_notification_date = 0; int32 last_notification_date = 0;
NotificationId last_notification_id; NotificationId last_notification_id;
@ -17859,6 +17868,9 @@ bool MessagesManager::add_new_message_notification(Dialog *d, Message *m, bool f
LOG_IF(WARNING, !have_settings) << "Have no notification settings for " << settings_dialog_id LOG_IF(WARNING, !have_settings) << "Have no notification settings for " << settings_dialog_id
<< ", but forced to send notification about " << m->message_id << " in " << ", but forced to send notification about " << m->message_id << " in "
<< d->dialog_id; << d->dialog_id;
// notification group must be preloaded to guarantee that there is no race between
// get_message_notifications_from_database and new notifications added right now
td_->notification_manager_->load_group_force(get_dialog_message_notification_group_id(d));
do { do {
m->notification_id = td_->notification_manager_->get_next_notification_id(); m->notification_id = td_->notification_manager_->get_next_notification_id();
} while (d->notification_id_to_message_id.count(m->notification_id) != 0); // just in case } while (d->notification_id_to_message_id.count(m->notification_id) != 0); // just in case
@ -17866,9 +17878,6 @@ bool MessagesManager::add_new_message_notification(Dialog *d, Message *m, bool f
add_notification_id_to_message_id_correspondence(d, m->notification_id, m->message_id); add_notification_id_to_message_id_correspondence(d, m->notification_id, m->message_id);
} }
bool is_changed = set_dialog_last_notification(d, m->date, m->notification_id, "add_new_message_notification"); bool is_changed = set_dialog_last_notification(d, m->date, m->notification_id, "add_new_message_notification");
if (!d->first_new_notification_id.is_valid()) {
d->first_new_notification_id = m->notification_id;
}
CHECK(is_changed); CHECK(is_changed);
VLOG(notifications) << "Create " << m->notification_id << " with " << m->message_id << " in " << d->dialog_id; VLOG(notifications) << "Create " << m->notification_id << " with " << m->message_id << " in " << d->dialog_id;
send_closure_later(G()->notification_manager(), &NotificationManager::add_notification, send_closure_later(G()->notification_manager(), &NotificationManager::add_notification,
@ -20644,6 +20653,12 @@ MessagesManager::Message *MessagesManager::add_message_to_dialog(Dialog *d, uniq
} }
} }
if (*need_update) {
// notification must be added before updating unread_count to have correct total notification count
// in get_message_notification_group_force
add_new_message_notification(d, message.get(), false);
}
UserId my_user_id(td_->contacts_manager_->get_my_id()); UserId my_user_id(td_->contacts_manager_->get_my_id());
DialogId my_dialog_id(my_user_id); DialogId my_dialog_id(my_user_id);
if (*need_update && message_id.get() > d->last_read_inbox_message_id.get() && !td_->auth_manager_->is_bot()) { if (*need_update && message_id.get() > d->last_read_inbox_message_id.get() && !td_->auth_manager_->is_bot()) {
@ -20700,10 +20715,6 @@ MessagesManager::Message *MessagesManager::add_message_to_dialog(Dialog *d, uniq
repair_channel_server_unread_count(d); repair_channel_server_unread_count(d);
} }
if (*need_update) {
add_new_message_notification(d, message.get(), false);
}
const Message *m = message.get(); const Message *m = message.get();
if (!m->from_database && !message_id.is_yet_unsent()) { if (!m->from_database && !message_id.is_yet_unsent()) {
add_message_to_database(d, m, "add_message_to_dialog"); add_message_to_database(d, m, "add_message_to_dialog");

View File

@ -888,7 +888,6 @@ class MessagesManager : public Actor {
int32 last_notification_date = 0; // last known date of last notification in the dialog int32 last_notification_date = 0; // last known date of last notification in the dialog
NotificationId last_notification_id; // last known identifier of last notification in the dialog NotificationId last_notification_id; // last known identifier of last notification in the dialog
NotificationId max_removed_notification_id; // notification identifier, up to which all notifications are removed NotificationId max_removed_notification_id; // notification identifier, up to which all notifications are removed
NotificationId first_new_notification_id; // identifier of first added notification in that library launch
bool has_contact_registered_message = false; bool has_contact_registered_message = false;

View File

@ -164,6 +164,11 @@ NotificationManager::NotificationGroups::iterator NotificationManager::get_group
return groups_.end(); return groups_.end();
} }
void NotificationManager::load_group_force(NotificationGroupId group_id) {
auto group_it = get_group_force(group_id, true);
CHECK(group_it != groups_.end());
}
NotificationManager::NotificationGroups::iterator NotificationManager::get_group_force(NotificationGroupId group_id, NotificationManager::NotificationGroups::iterator NotificationManager::get_group_force(NotificationGroupId group_id,
bool send_update) { bool send_update) {
auto group_it = get_group(group_id); auto group_it = get_group(group_id);

View File

@ -48,6 +48,8 @@ class NotificationManager : public Actor {
NotificationGroupId get_next_notification_group_id(); NotificationGroupId get_next_notification_group_id();
void load_group_force(NotificationGroupId group_id);
void add_notification(NotificationGroupId group_id, DialogId dialog_id, int32 date, void add_notification(NotificationGroupId group_id, DialogId dialog_id, int32 date,
DialogId notification_settings_dialog_id, bool is_silent, NotificationId notification_id, DialogId notification_settings_dialog_id, bool is_silent, NotificationId notification_id,
unique_ptr<NotificationType> type); unique_ptr<NotificationType> type);