To future googlers: It turns out this is a bad idea--you can have a packet sitting in session->packets but still not be able to process the data until more packets are available.
I added a "count" property to the list_head struct, incrementing it in _libssh2_list_add() and decrementing in _libssh2_list_remove(). In _libssh2_channel_read() instead of calling _libssh2_transport_read() once if session->packets is empty, I and session->packets->count is below a constant limit. (8 seems reasonable..?) A better solution might be to track the total size of the buffered data and place a limit on that instead.
diff -ru /Users/dave/Desktop/libssh2-1.4.2/src/channel.c libssh2-1.4.2/src/channel.c
--- /Users/dave/Desktop/libssh2-1.4.2/src/channel.c 2012-05-18 14:29:03.000000000 -0700
+++ libssh2-1.4.2/src/channel.c 2012-08-15 16:57:53.000000000 -0700
@@ -1760,10 +1760,12 @@
rc = 1; /* set to >0 to let the while loop start */
+#define MAX_BUFFERED_PACKETS 8
+
/* Process all pending incoming packets in all states in order to "even
out" the network readings. Tests prove that this way produces faster
transfers. */
- while (rc > 0)
+ while (rc > 0 && _libssh2_list_count(&session->packets) < MAX_BUFFERED_PACKETS)
rc = _libssh2_transport_read(session);
if ((rc < 0) && (rc != LIBSSH2_ERROR_EAGAIN))
diff -ru /Users/dave/Desktop/libssh2-1.4.2/src/misc.c libssh2-1.4.2/src/misc.c
--- /Users/dave/Desktop/libssh2-1.4.2/src/misc.c 2011-08-25 10:59:47.000000000 -0700
+++ libssh2-1.4.2/src/misc.c 2012-08-15 15:47:00.000000000 -0700
@@ -474,6 +474,7 @@
/* init the list head */
void _libssh2_list_init(struct list_head *head)
{
+ head->count = 0;
head->first = head->last = NULL;
}
@@ -498,6 +499,8 @@
entry->prev->next = entry;
else
head->first = entry;
+
+ ++head->count;
}
/* return the "first" node in the list this head points to */
@@ -518,9 +521,16 @@
return node->prev;
}
+int _libssh2_list_count(struct list_head *head)
+{
+ return head->count;
+}
+
/* remove this node from the list */
void _libssh2_list_remove(struct list_node *entry)
{
+ --entry->head->count;
+
if(entry->prev)
entry->prev->next = entry->next;
else
diff -ru /Users/dave/Desktop/libssh2-1.4.2/src/misc.h libssh2-1.4.2/src/misc.h
--- /Users/dave/Desktop/libssh2-1.4.2/src/misc.h 2012-05-14 10:50:53.000000000 -0700
+++ libssh2-1.4.2/src/misc.h 2012-08-15 15:09:34.000000000 -0700
@@ -39,6 +39,7 @@
*/
struct list_head {
+ int count;
struct list_node *last;
struct list_node *first;
};
@@ -66,6 +67,8 @@
/* return the prev node in the list */
void *_libssh2_list_prev(struct list_node *node);
+int _libssh2_list_count(struct list_head *head);
+
/* remove this node from the list */
void _libssh2_list_remove(struct list_node *entry);
On Jun 27, 2012, at 5:09 PM, Dave Hayden wrote:
> At the head of _libssh2_channel_read(), a while loop calls _libssh2_transport_read(), after this comment:
>
>> Process all pending incoming packets in all states in order to "even out" the network readings. Tests prove that this way produces faster transfers.
>
>
> That makes sense for file transfers where the server will wait for an acknowledgement, but in the case of a terminal session that's firehosing data at us faster than we can process, that winds up filling up all available memory--instead of just the network buffer. I changed that code to
>
> read_packet = _libssh2_list_first(&session->packets);
>
> while ( read_packet == NULL && rc > 0 )
> {
> rc = _libssh2_transport_read(session);
> read_packet = _libssh2_list_first(&session->packets);
> }
>
> and it appears to fix this problem. Maybe there should be a channel or session setting, choosing between the two? Or a limit on the number of packets the session will buffer?
>
> Thanks!
> -Dave
>
>
> _______________________________________________
> libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2012-08-16