From daa59b89b1f05cf3a2abdee9ef5ac8bffe805b13 Mon Sep 17 00:00:00 2001 From: dec05eba Date: Sat, 12 Jan 2019 20:39:23 +0100 Subject: Fix multithreading crashes --- depends/dchat_core | 2 +- include/ChatWindow.hpp | 4 +++- include/Window.hpp | 6 ++++-- src/ChatWindow.cpp | 39 ++++++++++++++++++--------------------- src/RoomNotificationsWindow.cpp | 3 ++- src/Window.cpp | 15 +++++++++------ src/WindowNotification.cpp | 4 ++-- src/main.cpp | 6 +++++- 8 files changed, 44 insertions(+), 35 deletions(-) diff --git a/depends/dchat_core b/depends/dchat_core index 9506378..b2e9deb 160000 --- a/depends/dchat_core +++ b/depends/dchat_core @@ -1 +1 @@ -Subproject commit 9506378c0423e4125c3ba208f5843e2c54a30715 +Subproject commit b2e9deb0e6733c0d79d6992809882e214dee7ed2 diff --git a/include/ChatWindow.hpp b/include/ChatWindow.hpp index 764ca32..1ca473b 100644 --- a/include/ChatWindow.hpp +++ b/include/ChatWindow.hpp @@ -65,6 +65,8 @@ namespace dchat { Gtk::Grid *leftPanelUsersLayout; Gtk::Grid *messageAreaLayout; + uint numMessages; + uint numUsers; Gtk::RadioButton *button; }; @@ -76,4 +78,4 @@ namespace dchat bool chatInputShowPlaceholder; bool chatInputChangeByPlaceholder; }; -} \ No newline at end of file +} diff --git a/include/Window.hpp b/include/Window.hpp index 645b578..b33605b 100644 --- a/include/Window.hpp +++ b/include/Window.hpp @@ -10,6 +10,7 @@ #include #include #include +#include #include namespace dchat @@ -33,6 +34,7 @@ namespace dchat void draw(const Cairo::RefPtr &cairo) { Gtk::Overlay::draw(cairo); } }; + std::thread::id mainThreadId; std::mutex databaseCallbackMutex; OverlayDrawable overlay; Gtk::Stack stack; @@ -58,7 +60,7 @@ namespace dchat sigc::connection drawBackgroundConnection; Glib::Dispatcher dispatcher; - std::mutex dispatcherMutex; + std::recursive_mutex dispatcherMutex; std::vector dispatcherHandlers; }; -} \ No newline at end of file +} diff --git a/src/ChatWindow.cpp b/src/ChatWindow.cpp index 9cc4ff1..9fc810b 100644 --- a/src/ChatWindow.cpp +++ b/src/ChatWindow.cpp @@ -184,8 +184,7 @@ namespace dchat window->windowNotification->show("Room name has to be between 1 and 32 characters"); else { - auto room = window->rooms->createRoom(roomName); - setCurrentRoom(room); + window->rooms->createRoom(roomName); } break; } @@ -384,7 +383,9 @@ namespace dchat leftPanelChannels.attach(*roomButton, 0, roomCount, 1, 1); ++roomCount; - RoomData *roomData = new RoomData { leftPanelUsersLayout, messageAreaLayout, roomButton }; + uint numMessages = 0; + uint numUsers = 0; + RoomData *roomData = new RoomData { leftPanelUsersLayout, messageAreaLayout, numMessages, numUsers, roomButton }; roomDataById[*room->id] = roomData; if(!currentRoom) { @@ -407,6 +408,7 @@ namespace dchat messageAreaStack.set_visible_child(roomIdStr); topbar->setTitle(room->name); currentRoom = room; + assert(roomDataById.find(*room->id) != roomDataById.end()); currentRoomData = roomDataById[*room->id]; if(room->localUser) { @@ -433,20 +435,15 @@ namespace dchat void ChatWindow::addMessage(const RoomAddMessageRequest &request) { - auto roomMessages = request.room->messages; - RoomMessage *lastMessage = nullptr; - if(!roomMessages.empty()) - lastMessage = &roomMessages.back(); - - if(lastMessage && lastMessage->creator->publicKey == request.message.creator->publicKey) + if(request.prevMessage && request.prevMessage->creator->publicKey == request.message->creator->publicKey) { - int64_t msgTimeDiff = (int64_t)request.message.timestampSeconds - (int64_t)lastMessage->timestampSeconds; + int64_t msgTimeDiff = (int64_t)request.message->timestampSeconds - (int64_t)request.prevMessage->timestampSeconds; if(msgTimeDiff <= MERGE_MESSAGE_TIMESTAMP_DIFF_SEC) { - auto message = messageById[lastMessage->id]; - message->text.set_text(message->text.get_text() + "\n" + request.message.text); + auto message = messageById[request.prevMessage->id]; + message->text.set_text(message->text.get_text() + "\n" + request.message->text); // Since messages that are sent withing a timeframe are combined, several message ids can refer to the same message - messageById[request.message.id] = message; + messageById[request.message->id] = message; if(!request.loadedFromCache && *request.room->id == *currentRoom->id) { currentRoomData->messageAreaLayout->queue_draw(); @@ -456,20 +453,20 @@ namespace dchat } } - std::string userNickname = request.message.creator->nickname; + std::string userNickname = request.message->creator->nickname; if(userNickname.empty()) userNickname = "Anonymous"; - ChatMessage *message = Gtk::manage(new ChatMessage(userNickname, request.message.text, request.message.timestampSeconds)); - if(request.message.creator->avatarUrl.empty()) + ChatMessage *message = Gtk::manage(new ChatMessage(userNickname, request.message->text, request.message->timestampSeconds)); + if(request.message->creator->avatarUrl.empty()) message->avatar.url = "https://discordemoji.com/assets/emoji/PeepoHide.png"; else - message->avatar.url = request.message.creator->avatarUrl; + message->avatar.url = request.message->creator->avatarUrl; message->set_valign(Gtk::Align::ALIGN_START); message->set_hexpand(true); message->show_all(); - messageById[request.message.id] = message; + messageById[request.message->id] = message; RoomData *roomData = roomDataById[*request.room->id]; - roomData->messageAreaLayout->attach(*message, 0, roomMessages.size(), 1, 1); + roomData->messageAreaLayout->attach(*message, 0, roomData->numMessages++, 1, 1); // TODO: When we get a message in the current room we scroll to the bottom, but this should only be done if we are not manually scrolling to view old messages if(!request.loadedFromCache && *request.room->id == *currentRoom->id) @@ -487,7 +484,7 @@ namespace dchat username->get_style_context()->add_class("username-list-username"); request.user->userdata = username; RoomData *roomData = roomDataById[*request.room->id]; - roomData->leftPanelUsersLayout->attach(*username, 0, request.room->userByPublicKey.size() - 1, 1, 1); + roomData->leftPanelUsersLayout->attach(*username, 0, roomData->numUsers++, 1, 1); fprintf(stderr, "Added user %s\n", request.user->publicKey.toString().c_str()); if(roomData == currentRoomData) @@ -555,4 +552,4 @@ namespace dchat auto adj = messageArea.get_vadjustment(); adj->set_value(adj->get_upper()); } -} \ No newline at end of file +} diff --git a/src/RoomNotificationsWindow.cpp b/src/RoomNotificationsWindow.cpp index a491840..ba47836 100644 --- a/src/RoomNotificationsWindow.cpp +++ b/src/RoomNotificationsWindow.cpp @@ -48,6 +48,7 @@ namespace dchat dialog.get_content_area()->pack_start(*Gtk::manage(new Gtk::Label(msg))); dialog.add_button("Yes", Gtk::RESPONSE_YES); dialog.add_button("No", Gtk::RESPONSE_NO); + dialog.show_all(); switch(dialog.run()) { case Gtk::RESPONSE_YES: @@ -102,4 +103,4 @@ namespace dchat row[roomNotifications->userPublicKeyColumn] = userPublicKeyStr; row[roomNotifications->messageColumn] = request.message; } -} \ No newline at end of file +} diff --git a/src/Window.cpp b/src/Window.cpp index 255986a..8a6652e 100644 --- a/src/Window.cpp +++ b/src/Window.cpp @@ -41,7 +41,7 @@ namespace dchat return result; } - Window::Window() : + Window::Window() : chatWindow(this) { set_border_width(0); @@ -230,12 +230,15 @@ namespace dchat dispatcher.connect([this]() { - std::lock_guard lock(dispatcherMutex); - for(DispatcherHandler &dispatcherHandler : dispatcherHandlers) + std::lock_guard lock(dispatcherMutex); + // We make a copy because dispatcherHandler below can call main loop again (for example when scrolling) which will call this function, + // and we want to prevent the same dispatcher from being called multiple times + auto handlersCopies = dispatcherHandlers; + dispatcherHandlers.clear(); + for(DispatcherHandler &dispatcherHandler : handlersCopies) { dispatcherHandler(); } - dispatcherHandlers.clear(); }); Rooms::connect(bootstrapNode->c_str(), 27130, roomCallbackFuncs); @@ -280,7 +283,7 @@ namespace dchat void Window::dispatchFunction(DispatcherHandler func) { - std::lock_guard lock(dispatcherMutex); + std::lock_guard lock(dispatcherMutex); dispatcherHandlers.push_back(func); dispatcher.emit(); } @@ -380,4 +383,4 @@ namespace dchat queue_draw(); return true; } -} \ No newline at end of file +} diff --git a/src/WindowNotification.cpp b/src/WindowNotification.cpp index 9e17108..84365e9 100644 --- a/src/WindowNotification.cpp +++ b/src/WindowNotification.cpp @@ -29,7 +29,7 @@ namespace dchat label.set_text(text); set_visible(true); revealer.set_reveal_child(true); - int showTime = (int)std::max(1.0, (double)text.size() * 0.1) * 1000; + unsigned int showTime = (int)std::max(1.0, (double)text.size() * 0.1) * 1000; hideTimer = Glib::signal_timeout().connect([this] { @@ -43,4 +43,4 @@ namespace dchat return false; }, showTime + revealer.get_transition_duration()); } -} \ No newline at end of file +} diff --git a/src/main.cpp b/src/main.cpp index f6b2ae6..b985d53 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -6,6 +6,10 @@ #include #endif +// TODO: When creating a room, immediately switch to it. +// TODO: When changing nickname or avatar, do not merge next message with previous one even if next message is sent right after the previous one. +// We want to see user changes immediately. + static int setWindowCss(dchat::Window &window, Glib::RefPtr css) { auto ctx = window.get_style_context(); @@ -32,7 +36,7 @@ static int setWindowCss(dchat::Window &window, Glib::RefPtr cs int main (int argc, char *argv[]) { - auto app = Gtk::Application::create(argc, argv, "dec05eba.dchat"); + auto app = Gtk::Application::create(argc, argv, "dec05eba.dchat", Gio::APPLICATION_NON_UNIQUE); auto css = Gtk::CssProvider::create(); dchat::Window window; if(setWindowCss(window, css) != 0) -- cgit v1.2.3