Summary

Fix hang when completing a file transfer over XMPP

Review Request #1466 — Created May 21, 2022 and submitted May 26, 2022, 4:51 a.m.

Information

Owner

belgin

Repository

pidgin/pidgin

Branch

release-2.x.y

Bugs

Depends On

Reviewers

Groups

pidgin

People

Description

The way purple_xfer_drain_socket was implemented,
if the peer never close()d the connection, we would
wait forever for it to be closed remotely, causing
Pidgin to hang.

Testing Done

Tested IRC and XMPP on Windows and Linux. Transferred big and small files.

Commits

Summary	ID
Fix hang when completing a file transfer over XMPP The way purple_xfer_drain_socket was implemented, if the peer never close()d the connection, we would wait forever for it to be closed remotely, causing Pidgin to hang. Testing Done: Tested IRC and XMPP on Windows and Linux. Transferred big and small files. Reviewed at https://reviews.imfreedom.org/r/1466/	613b2af77c7a0e32abda71c2bb9eb14942c076f9

Issues

Description	From	Last Updated
extra whitespace	grim	May 23, 2022, 9:33 p.m.
{'s please	grim	May 23, 2022, 9:33 p.m.
Can you comment a bit on exactly what you're testing for in each of these conditionals? Like I know it's …	grim	May 23, 2022, 9:33 p.m.
operators should have whitespace around them.	grim	May 24, 2022, 5:51 p.m.
whitespace around the + again.	grim	May 24, 2022, 5:51 p.m.

Change Summary:

Turns out there was no need to drain the socket before closing it at all.

IRC transfers were failing because purple_xfer was closing the connection from the sender's side

before the receiver read everything.

The solution was in two parts: 

Make purple_xfer close the connections automatically only when

there is no read/write function defined in the respective protocol.
Make a read function for IRC DCC that handles a return value of 0 

from read() gracefully.

This should be tested with more protocols to see if the transfers finish correctly.

Commits:

	Summary	ID
	Fix hang when completing a file transfer over XMPP The way purple_xfer_drain_socket was implemented, if the peer never close()d the connection, we would wait forever for it to be closed remotely, causing Pidgin to hang.	04c578355d4f85dcec11c148215c0176b71aea64
	Fix hang when completing a file transfer over XMPP The way purple_xfer_drain_socket was implemented, if the peer never close()d the connection, we would wait forever for it to be closed remotely, causing Pidgin to hang.	58121aa13583f527b4f14b8f1dc89f7b2a9be83f

Diff:

Revision 2 (+82 -44)

Show changes

	libpurple/ft.c
	libpurple/protocols/irc/dcc_send.c

libpurple/protocols/irc/dcc_send.c (Diff revision 2)
The issue has been resolved. Show all issues
```
extra whitespace
```
libpurple/protocols/irc/dcc_send.c (Diff revision 2)
The issue has been resolved. Show all issues
```
{'s please
```

libpurple/protocols/irc/dcc_send.c (Diff revision 2)

The issue has been resolved. Show all issues

Can you comment a bit on exactly what you're testing for in each of these conditionals? Like I know it's how many bytes were read, but like a little more explanation wouldn't hurt especially around the r == 0 block.

Change Summary:

Address issues.

Commits:

	Summary	ID
	Fix hang when completing a file transfer over XMPP The way purple_xfer_drain_socket was implemented, if the peer never close()d the connection, we would wait forever for it to be closed remotely, causing Pidgin to hang.	58121aa13583f527b4f14b8f1dc89f7b2a9be83f
	Fix hang when completing a file transfer over XMPP The way purple_xfer_drain_socket was implemented, if the peer never close()d the connection, we would wait forever for it to be closed remotely, causing Pidgin to hang.	ed91cc38f8f286a116388cf1ae23d7868d304424

Diff:

Revision 3 (+92 -42)

Show changes

	libpurple/ft.c
	libpurple/protocols/irc/dcc_send.c

libpurple/ft.c (Diff revision 3)
The issue has been resolved. Show all issues
```
operators should have whitespace around them.
```
libpurple/ft.c (Diff revision 3)
The issue has been resolved. Show all issues
```
whitespace around the + again.
```

Change Summary:

Address issues.

Commits:

	Summary	ID
	Fix hang when completing a file transfer over XMPP The way purple_xfer_drain_socket was implemented, if the peer never close()d the connection, we would wait forever for it to be closed remotely, causing Pidgin to hang.	ed91cc38f8f286a116388cf1ae23d7868d304424
	Fix hang when completing a file transfer over XMPP The way purple_xfer_drain_socket was implemented, if the peer never close()d the connection, we would wait forever for it to be closed remotely, causing Pidgin to hang. Testing Done: Tested IRC and XMPP on Windows and Linux. Transferred big and small files. Reviewed at https://reviews.imfreedom.org/r/1466/	613b2af77c7a0e32abda71c2bb9eb14942c076f9

Diff:

Revision 4 (+92 -42)

Show changes

	libpurple/ft.c
	libpurple/protocols/irc/dcc_send.c

Ship it!

```
Ship It!
```
```
Awesome work, thank you very much!!
```

Status:: Completed