Skip to content
This repository has been archived by the owner on Feb 12, 2023. It is now read-only.

Messages are sent to me twice on some occasions #2726

Closed
ghost opened this issue Dec 22, 2015 · 67 comments
Closed

Messages are sent to me twice on some occasions #2726

ghost opened this issue Dec 22, 2015 · 67 comments
Labels
C-bug The issue contains a bug report upstream The problem is with a component from a 3rd party

Comments

@ghost
Copy link

ghost commented Dec 22, 2015

I have recently noticed (after one of the last updates) that when somebody sends me a message, about 2 or 3 seconds after they have sent it they send me an exact duplicate, however this does not happen with all messages they send me, nor do I see it in relation to any of my messages. The other strange thing is that the other person cannot see the duplicate message and only sees the original.

Here is an example of a duplicate message I see:

duplicate_messages_qtox

@ghost
Copy link
Author

ghost commented Dec 22, 2015

Happens to me also, running latest git version

@ProMcTagonist
Copy link
Contributor

Same.

@ElLamparto
Copy link

If the ACK is lost for any reason, the message is resent. Not much to do about it.

@ghost
Copy link
Author

ghost commented Dec 23, 2015

If a message needs to be resent though can't it send some information with it that tells the qTox client that it has been resent, and then if the qTox client registers that there is another exact duplicate message already being displayed, and this is an automatic resend, and not what the user has requested, then just not to display the duplicate message?

@agilob
Copy link
Contributor

agilob commented Dec 23, 2015

You should discuss this in toxcore repo.

@ghost
Copy link
Author

ghost commented Dec 23, 2015

I have filed a bug report on this here.

@tux3 tux3 added the C-bug The issue contains a bug report label Dec 31, 2015
@tux3 tux3 added the upstream The problem is with a component from a 3rd party label Jan 18, 2016
@LittleVulpix
Copy link
Contributor

People keep saying this is a toxcore issue and it is an extension thereof, but I believe qtox is "worse than others" at sending double messages. All of my friends that have qtox occassionally send me duplicate messages. Whereas I (using miranda NG) and some of my uTox friends NEVER send me duplicate messages.

There must be some kind of timeout in qtox that tries to resend a message if an ack(?) is not received within a given timeframe. I would recommend extending this timeout to at least 15-20 seconds. It is somewhat annoying to keep getting duplicate messages from qTox users.

@ghost
Copy link
Author

ghost commented Mar 22, 2016

Though it may be something else, this may be completely failing here #3022.

@zetok
Copy link
Contributor

zetok commented Mar 22, 2016

(…) I believe qtox is "worse than others" at sending double messages. (…)

There must be some kind of timeout in qtox that tries to resend a message if an ack(?) is not received within a given timeframe. I would recommend extending this timeout to at least 15-20 seconds. It is somewhat annoying to keep getting duplicate messages from qTox users.

Right, there is timeout, and IIRC it's a bit short. Increasing it might prove helpful, even if it won't fix the underlying issue.

@ProMcTagonist
Copy link
Contributor

We need numbered messages in core. It's been talked about but probably not focused on because of new groups.

@zetok Do you know what the current timeout is? I agree it should be at least 15 seconds.

@zetok
Copy link
Contributor

zetok commented Mar 23, 2016

@ProMcTagonist Um, from what I can see, timeout is currently 15s.

I'll make PR doubling it, or perhaps it should be 45s ?

@zetok
Copy link
Contributor

zetok commented Mar 23, 2016

Actually, I'm not sure if that's it. It's either of 2 values, and that would be either 2s, or 15s. Well, PR will need to be tested.
Eh, if only qTox code had actual comments :/

zetok added a commit to zetok/qTox that referenced this issue Mar 23, 2016
Should make problem with duplicated messages less common.
Related to qTox#2726.
@ddobrev
Copy link

ddobrev commented Apr 14, 2016

I don't know if the workaround has entered the "stable" 1.3.0 version but this happens every time there is a minor problem with a network packet and the message cannot therefore be immediately sent. I have received 3 times a group of 5 messages and a friend has received a message 6 times. If you cannot code it on your own, then please use an external library such as http://mqtt.org/.
This problem is one of the few (another would be #2820) that affect qTox any configuration on any OS. Another similarly to them is that nothing seems to be done about it. Can we trust qTox at all?

@ElLamparto
Copy link

I use qTox 24/24, 7/7 and message repetition happens almost never.
Freezing video on Linux (regression!) is the real problem.

@ProMcTagonist
Copy link
Contributor

I haven't shut qTox, except to apply upgrades, for a long time. I'm on Linux and I get message repetition every few days.

anoadragon453 pushed a commit to anoadragon453/qTox that referenced this issue Apr 25, 2016
Should make problem with duplicated messages less common.
Related to qTox#2726.
@LittleVulpix
Copy link
Contributor

LittleVulpix commented May 17, 2016

This is still happening even on the latest qtox. None of the other clients ever send me duplicate messages, just qtox. After restarting qtox, issue is usually gone but I dunno if it's because of any bugs in qtox. I think it's gone because of fresh connections being made/remade...

Usually the duplicate arrives within 10 seconds, but I had one case where it took 30 seconds.
I can say however that I no longer get triplicate messages, which is an improvement..

Assume A is using qtox 1.4.1 latest version available for the updater.

See two actual examples below:

A, 17.05.2016 09:36:07:
??

A, 09:36:10:
??

and one more

Vulpix, 17.05.2016 09:36:27:
respond with "bla" once when you see this

A, 09:36:35:
bla

A, 09:36:40:
bla

and one more where the time is actually 30 seconds...

A, 17.05.2016 17:08:44:
is that a rhetorical question?

Vulpix, 17:09:11:
An actual one :p

A, 17:09:14:
is that a rhetorical question?

Here it's 30 seconds...

zetok added a commit to zetok/qTox that referenced this issue May 17, 2016
Should make duplicated messages even less common (qTox#2726)
zetok added a commit to zetok/qTox that referenced this issue May 17, 2016
Should make duplicated messages even less common (qTox#2726)
Increased ~proportionally to change in
76d8e19
@ddobrev
Copy link

ddobrev commented Sep 15, 2016

Is there any chance the pull requests could be put off in favour of this bug? I think they shouldn't've been started to begin with given this and other bugs which regularly annoy the hell out of users, along with other bugs. Why didn't you fix the bugs first so that people can have a decent experience while you refactor the code?

@GrayHatter
Copy link

This would be considered a high priority bug on uTox... Perhaps it's time
for a different client?

On Thu, Sep 15, 2016 at 11:30 AM, Dimitar Dobrev notifications@github.com
wrote:

Is there any chance the pull requests could be put off in favour of this
bug? I think they shouldn't've been started to begin with given this and
other bugs which regularly annoy the hell out of users, along with other
bugs. Why didn't you fix the bugs first so that people can have a decent
experience while you refactor the code?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2726 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAO20FQIV1a9kRa8jsanasKHPnOqcxBwks5qqY7igaJpZM4G575x
.

@ddobrev
Copy link

ddobrev commented Sep 15, 2016

@GrayHatter perhaps... it seems that bugs are not important to the qTox devs. Is uTox compatible with existing qTox profiles?

@zetok
Copy link
Contributor

zetok commented Sep 15, 2016

@GrayHatter have you fixed yet bug in µTox that causes whole xorg session to slow down when receiving messages? Seems like something that should have extremely high priority..

@ddobrev

Why didn't you fix the bugs first so that people can have a decent experience while you refactor the code?

You could ask @GrayHatter, he's the one who said that he has knowledge how to fix this bug and that fix could be made very quickly (5 min if memory serves well). Instead he chooses to act like an asshole, trying to promote crappy µTox at the cost of qTox.

The bugs are fixed by people who can reproduce them – in the last month or so I might have run into it like once.

Is there any chance the pull requests could be put off in favour of this bug?

How exactly not fixing other bugs would help fix this one?

@GrayHatter
Copy link

@ddobrev It should be, unless qTox is doing something shifty...

@zetok I've still never been able to duplicate that bug. But I think I know more about how to fix it, so if you do ever find a way for me to duplicate it, let me know so I can take a crack at it.

And yeah, the fix for that bug is kinda obvious, I don't know how qTox works well enough to open a PR myself. So it wouldn't be 5 min for me. (I also probably shouldn't say 5 minutes either... I don't know how much code would need to be changed)

@antis81
Copy link
Contributor

antis81 commented Sep 16, 2016

@GrayHatter

And yeah, the fix for that bug is kinda obvious…

Uhm… what do you mean by that? Do you know, where the problem is? Could you provide some pseudo c++ code without internal qTox knowledge required to clarify that?

@zetok
Copy link
Contributor

zetok commented Sep 20, 2016

relevant part from IRC:

[17:51:25] <grayhatter> first... delete this https://github.com/qTox/qTox/blob/master/src/persistence/offlinemsgengine.cpp#L38
[17:52:03] <grayhatter> then delete this
[17:52:04] <grayhatter> https://github.com/qTox/qTox/blob/master/src/persistence/offlinemsgengine.cpp#L102-L106
[17:52:32] <grayhatter> then take everything left here https://github.com/qTox/qTox/blob/master/src/persistence/offlinemsgengine.cpp#L93-L114
[17:52:39] <grayhatter> https://github.com/qTox/qTox/blob/master/src/core/corefile.cpp#L472
[17:52:43] <grayhatter> and put it here
[17:52:54] <grayhatter> that's how you fix the duplicate message bug in qTox
[17:53:13] <grayhatter> do you understand what I'm saying you should do?
[17:55:50] <grayhatter> rip uTox
[17:56:10] <sudden6> i'll look into it
[17:56:30] <grayhatter> do you understand my suggestion though?
[17:58:24] <sudden6> move offline message handling into connection status changed stuff?
[18:00:21] <grayhatter> correct
[18:00:32] <grayhatter> there's no reason to resend messages while the friend is still connected
[18:01:07] <grayhatter> toxcore promises every lossless packet you send will get there, or you'll get a connection dropped callback
[18:01:19] <grayhatter> then you just have to resend once the peer comes back online

@ddobrev
Copy link

ddobrev commented Sep 21, 2016

@zetok @antis81 does the excerpt above contain enough information for the bug to be fixed? That is, shall we see any work on it any time soon?

@antis81
Copy link
Contributor

antis81 commented Sep 22, 2016

does the excerpt above contain enough information for the bug to be fixed?…

It does and it gets even better. The PR #3639 further improves the situation, because unsent (!) messages are sent each time the chat opens (depending on the friend's online status) -> correct initialization.

…That is, shall we see any work on it any time soon?

I can provide the PR for the fix.

@antis81
Copy link
Contributor

antis81 commented Sep 22, 2016

@GrayHatter Thanks for the information via chat! Though corefile.cpp#L472 is the wrong place, but i get what you mean.

We provided initialization "incidentally" via #3639, as instead of creating every chat widget "dumb" on startup, we create it, when a chat is "activated" (a friend is selected). This in turn calls Friend::deliverOfflineMsgs() (not yet available in master), if the friend's status is anything but "offline". However, this is still WIP and some topics (especially group chats) are not finished yet (as what users would expect)…

@ddobrev
Copy link

ddobrev commented Sep 22, 2016

@antis81 thank you for the fix, I hope you'll have little trouble merging it.

@ddobrev
Copy link

ddobrev commented Oct 24, 2016

Hello all. I'd like to once again ask you what's happening with this project and if it's ever going to be fully useful. This bug has not been fixed for a year. #2820 , #3158 and #2771 have all seen zero attention as well. The all mighty refactoring at https://github.com/qTox/qTox/tree/ui/redesign hasn't been touched in a month. I do believe it's about time you either fixed some of them or added a large bold text at the top of your README saying "this project is just our playground, if it so happens that it works for you, fine by us".

@zetok
Copy link
Contributor

zetok commented Oct 24, 2016

@ddobrev not quite in readme, but license states it quite clearly: https://github.com/qTox/qTox/blob/master/LICENSE

I've been thinking about adding a license badge on top of README.md, I'll make sure to do that right after #3793 gets merged.

@ElLamparto
Copy link

@zetok, @ddobrev is just angry, that's all.

@anthonybilinski
Copy link
Member

Still see this issue built off tip at e606d3c
Comes up when pasting large blocks of text that are split into multiple messages or when sending rapid messages back to back.

The long timeout before resend might actually make the problem worse for me since instead of just getting the same message twice, you get the message once and then again 2 minutes later, interrupting whatever you were talking about at the time.

@anthonybilinski
Copy link
Member

Looked into this for a bit:
in ChatForm::SendMessageStr, core->sendAction is called, which sends the message and gives us the reciept. Then, profile->getHistory()->addNewMessage is called, which enques statements into the db to register the receipt, and some other stuff.

If your computer load is low, list of queues is small, and network latency is high, you then service the list of queues, add the receipt to the map, get the receipt back from your peer, and everything if fine.

If the load is high, list of queues is large, and network latency is low, you then might receive the receipt from your peer, which is silently discarded in OfflineMsgEngine::dischargeReceipt if not registered. You then service the list of commands, register the receipt, pend with a little spinny wheel forever, resend the message, get the ack, confuse your peer.

Changing History::addNewMessage from db->execLater to db->execNow fixes the problem completely for me. I can spam to my hearts content over LAN.

I'll test it tomorrow when I'm in person (currently remoting to a desktop) and can see how the blocking execute affects feel/responsiveness of the applciation, and prepare a PR.
I'll also remove a double handling of receipts (we currently try to remove each receipt twice, once from chatform registering receipts and once from nexus registering receipts), add a log when we receive a receipt not expected, and decrease our retry timeout from the time it is now (2 minutes) to something lower since I think it was only increased to as an attempt to fix this problem.

anthonybilinski added a commit to anthonybilinski/qTox that referenced this issue Aug 30, 2017
Fixes qTox#2726
Register for receipt handling only once, log if receipt is received that is unexpected, synchronously add receipt of a sent message to map so that the peer can't answer before we are expecting the receipt.
@anthonybilinski
Copy link
Member

This does hurt responsiveness, if you spam like crazy while building or something, it locks up for a few seconds. In normal use it pauses for ~200ms after you send a message. This still seems a lot better for me, since I was getting double messages multiple times a day which confuses conversations, but a bigger refactor to record the IDs synchronously without a db access.

uTox doesn't have this even after enabling history which is off by default.

anthonybilinski added a commit to anthonybilinski/qTox that referenced this issue Aug 30, 2017
Fixes qTox#2726
Register for receipt handling only once, log if receipt is received that is unexpected, synchronously add receipt of a sent message to map so that the peer can't answer before we are expecting the receipt.
anthonybilinski added a commit to anthonybilinski/qTox that referenced this issue Aug 30, 2017
Fixes qTox#2726
Register for receipt handling only once, log if receipt is received that is unexpected, synchronously add receipt of a sent message to map so that the peer can't answer before we are expecting the receipt.
anthonybilinski added a commit to anthonybilinski/qTox that referenced this issue Aug 31, 2017
Fixes qTox#2726
Register for receipt handling only once, log if receipt is received that is unexpected, synchronously add receipt of a sent message to map so that the peer can't answer before we are expecting the receipt.
@anthonybilinski
Copy link
Member

Fixed responsiveness, back to async, still safe #4608

anthonybilinski added a commit to anthonybilinski/qTox that referenced this issue Sep 1, 2017
Fixes qTox#2726
Register for receipt handling only once, cache receipts that are received before message is writen to history and mark a message as sent once both its receipt has been received and it has been writen to history
anthonybilinski added a commit to anthonybilinski/qTox that referenced this issue Sep 5, 2017
Fixes qTox#2726
Register for receipt handling only once, cache receipts that are received before message is writen to history and mark a message as sent once both its receipt has been received and it has been writen to history
anthonybilinski added a commit to anthonybilinski/qTox that referenced this issue Sep 5, 2017
Fixes qTox#2726
Register for receipt handling only once, cache receipts that are received before message is writen to history and mark a message as sent once both its receipt has been received and it has been writen to history
anthonybilinski added a commit to anthonybilinski/qTox that referenced this issue Sep 7, 2017
Fixes qTox#2726
Register for receipt handling only once, cache receipts that are received before message is writen to history and mark a message as sent once both its receipt has been received and it has been writen to history
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C-bug The issue contains a bug report upstream The problem is with a component from a 3rd party
Projects
None yet
Development

No branches or pull requests