Bug 48084 - Consumes 100% CPU when SSH-forwarded connection is refused
Summary: Consumes 100% CPU when SSH-forwarded connection is refused
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: idle (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://cgit.collabora.com/git/user/wj...
Whiteboard: review+
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-03-30 02:26 UTC by Will Thompson
Modified: 2012-04-02 09:40 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Will Thompson 2012-03-30 02:26:57 UTC
I run irssi in screen on a remote server, with the irssiproxy plugin listening on localhost:12345. I then ssh to that server with “LocalForward 12345 localhost:12345”, and tell telepathy-idle to connect to localhost:12345 on my laptop. (I'll refer to the two differing ideas of “localhost” as “laptop” and “vm” to avoid confusion.)

If the irssiproxy plugin isn't loaded, then Idle eats my CPU and never finishes connecting. Here's what seems to happen:

• Idle establishes a connection to laptop:12345.
• The ssh client accepts the connection, and forwards it.
• ssh on the vm tries to connect to vm:12345, which fails because nothing is listening (“channel 4: open failed: connect failed: Connection refused”, says SSH).
• Meanwhile, Idle transport is established:



** (telepathy-idle:27876): DEBUG: change_state: emitting status-changed, state 2, reason 1
** (telepathy-idle:27876): DEBUG: sconn_status_changed_cb: called with state 2

[Thread 0x7ffff381d700 (LWP 27886) exited]
** (telepathy-idle:27876): DEBUG: msg_queue_timeout_cb: called
** (telepathy-idle:27876): DEBUG: idle_server_connection_send_async: sending "PASS topsecr3t
" to OutputStream 0x656c00
** (telepathy-idle:27876): DEBUG: msg_queue_timeout_cb: called
** (telepathy-idle:27876): DEBUG: idle_server_connection_send_async: sending "NICK wjt
" to OutputStream 0x656c00
** (telepathy-idle:27876): DEBUG: _write_ready: g_output_stream_write failed : Error sending data: Broken pipe
** (telepathy-idle:27876): DEBUG: _msg_queue_timeout_ready: idle_server_connection_send failed: Error sending data: Broken pipe
** (telepathy-idle:27876): DEBUG: msg_queue_timeout_cb: called
** (telepathy-idle:27876): DEBUG: idle_server_connection_send_async: sending "NICK wjt
" to OutputStream 0x656c00
** (telepathy-idle:27876): DEBUG: _write_ready: g_output_stream_write failed : Error sending data: Broken pipe
** (telepathy-idle:27876): DEBUG: _msg_queue_timeout_ready: idle_server_connection_send failed: Error sending data: Broken pipe

and so on, ad infinitum.

So the issue seems to be that IdleServerConnection does not blow itself up if g_output_stream_write_finish() fails, and nor does IdleConnection. The latter restores the unsent message to the queue to try again later.

The 100% CPU usage is caused by g_input_stream_read_finish() returning 0—which, per the documentation for g_input_stream_read_async(), means EOF:

> On success, the number of bytes read into the buffer will be passed
> to the callback. It is not an error if this is not the same as the
> requested size, as it can happen e.g. near the end of a file, but
> generally we try to read as many bytes as requested. Zero is returned
> on end of file (or if count is zero), but never otherwise.

So here are a few things which could/should be done:

• IdleServerConnection should blow itself up if a write fails (or IdleConnection could do that);
• IdleConnection should not try to requeue failed messages—it should just end the connection, one way or another.
• IdleServerConnection should end the connection if g_input_stream_read_finish() returns 0.

Let's see…
Comment 1 Will Thompson 2012-03-30 05:28:02 UTC
(In reply to comment #0)
> • IdleServerConnection should blow itself up if a write fails (or
> IdleConnection could do that);

I made IdleConnection blow up the connection on encountering IdleServerConnection::status-changed.

> • IdleConnection should not try to requeue failed messages—it should just end
> the connection, one way or another.

I removed all the re-queueing logic; I think the IdleServerConnection::status-changed handler will do the job for blowing the connection away.

> • IdleServerConnection should end the connection if
> g_input_stream_read_finish() returns 0.

Yup, it now emits ::status-changed in this case.

2087 % git diff --stat master src
 src/idle-connection.c        |  154 ++++++++++-------------------------------
 src/idle-server-connection.c |   18 +++++-
 2 files changed, 55 insertions(+), 117 deletions(-)

Not bad.
Comment 2 Jonny Lamb 2012-03-30 12:09:10 UTC
(In reply to comment #1)
> 2087 % git diff --stat master src
>  src/idle-connection.c        |  154 ++++++++++-------------------------------
>  src/idle-server-connection.c |   18 +++++-
>  2 files changed, 55 insertions(+), 117 deletions(-)
> 
> Not bad.

Good.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.