From 7de81270a35e3867a5593e44c11f85a5c4b93e09 Mon Sep 17 00:00:00 2001
From: Ivan Grokhotkov <ivan@espressif.com>
Date: Mon, 8 May 2017 15:27:51 +0800
Subject: [PATCH] =?UTF-8?q?ClientContext:=20don=E2=80=99t=20read=20DataSou?=
 =?UTF-8?q?rce=20in=20TCP=20callback?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, _write_some function would be called each time TCP stack
notifies the application that some data was delivered (via the `sent`
callback). In turn, _write_some would obtain more data to be sent from
the DataSource. In case of a DataSource backed by a Stream, this would
read from a stream. Some libraries (such as SD) may call `yield` and
other blocking operations from Stream read function, which can not be
used in TCP stack callbacks.

This change moves the data sending loop back into the Arduino task, with
a negligible loss of performance. TCP callback now wakes the main task
via `esp_schedule`, which performs stream read and provides more data
to the TCP stack.

Possible future optimization would be to buffer Stream data ahead of
time. That way, buffered data could be sent immediately from the TCP
callback. On the other hand, this optimization would need extra TCP_MSS
of temporary storage, per connection.

Fixes #2399.
---
 .../ESP8266WiFi/src/include/ClientContext.h   | 49 ++++++++++++-------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/libraries/ESP8266WiFi/src/include/ClientContext.h b/libraries/ESP8266WiFi/src/include/ClientContext.h
index 7a357390d..c7d5bde00 100644
--- a/libraries/ESP8266WiFi/src/include/ClientContext.h
+++ b/libraries/ESP8266WiFi/src/include/ClientContext.h
@@ -317,9 +317,7 @@ protected:
 
     void _cancel_write()
     {
-        if (_datasource) {
-            delete _datasource;
-            _datasource = nullptr;
+        if (_send_waiting) {
             esp_schedule();
         }
     }
@@ -327,14 +325,22 @@ protected:
     size_t _write_from_source(DataSource* ds)
     {
         assert(_datasource == nullptr);
+        assert(_send_waiting == 0);
         _datasource = ds;
         _written = 0;
-        _write_some();
-        while (_datasource && !_noblock) {
-            _send_waiting = true;
+        do {
+            _write_some();
+
+            if (!_datasource->available() || _noblock || state() == CLOSED) {
+                delete _datasource;
+                _datasource = nullptr;
+                break;
+            }
+
+            ++_send_waiting;
             esp_yield();
-        }
-        _send_waiting = false;
+        } while(true);
+        _send_waiting = 0;
         return _written;
     }
 
@@ -350,31 +356,36 @@ protected:
             can_send = 0;
         }
         size_t will_send = (can_send < left) ? can_send : left;
-        bool did_write = false;
-        while( will_send ) {
+        DEBUGV(":wr %d %d %d\r\n", will_send, left, _written);
+        bool need_output = false;
+        while( will_send && _datasource) {
             size_t next_chunk =
                 will_send > _write_chunk_size ? _write_chunk_size : will_send;
             const uint8_t* buf = _datasource->get_buffer(next_chunk);
+            if (state() == CLOSED) {
+                need_output = false;
+                break;
+            }
             err_t err = tcp_write(_pcb, buf, next_chunk, TCP_WRITE_FLAG_COPY);
+            DEBUGV(":wrc %d %d %d\r\n", next_chunk, will_send, err);
             _datasource->release_buffer(buf, next_chunk);
             if (err == ERR_OK) {
                 _written += next_chunk;
-                did_write = true;
+                need_output = true;
+            } else {
+                break;
             }
             will_send -= next_chunk;
         }
-        if( did_write ) tcp_output(_pcb);
-
-        if (!_datasource->available() || _noblock) {
-            delete _datasource;
-            _datasource = nullptr;
+        if( need_output ) {
+            tcp_output(_pcb);
         }
     }
 
     void _write_some_from_cb()
     {
-        _write_some();
-        if (!_datasource && _send_waiting) {
+        if (_send_waiting == 1) {
+            _send_waiting--;
             esp_schedule();
         }
     }
@@ -490,7 +501,7 @@ private:
     size_t _written = 0;
     size_t _write_chunk_size = 256;
     bool _noblock = false;
-    bool _send_waiting = false;
+    int _send_waiting = 0;
 };
 
 #endif//CLIENTCONTEXT_H