Skip to content

Commit 7de8127

Browse files
committed
ClientContext: don’t read DataSource in TCP callback
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 esp8266#2399.
1 parent 03baea2 commit 7de8127

File tree

1 file changed

+30
-19
lines changed

1 file changed

+30
-19
lines changed

libraries/ESP8266WiFi/src/include/ClientContext.h

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -317,24 +317,30 @@ class ClientContext
317317

318318
void _cancel_write()
319319
{
320-
if (_datasource) {
321-
delete _datasource;
322-
_datasource = nullptr;
320+
if (_send_waiting) {
323321
esp_schedule();
324322
}
325323
}
326324

327325
size_t _write_from_source(DataSource* ds)
328326
{
329327
assert(_datasource == nullptr);
328+
assert(_send_waiting == 0);
330329
_datasource = ds;
331330
_written = 0;
332-
_write_some();
333-
while (_datasource && !_noblock) {
334-
_send_waiting = true;
331+
do {
332+
_write_some();
333+
334+
if (!_datasource->available() || _noblock || state() == CLOSED) {
335+
delete _datasource;
336+
_datasource = nullptr;
337+
break;
338+
}
339+
340+
++_send_waiting;
335341
esp_yield();
336-
}
337-
_send_waiting = false;
342+
} while(true);
343+
_send_waiting = 0;
338344
return _written;
339345
}
340346

@@ -350,31 +356,36 @@ class ClientContext
350356
can_send = 0;
351357
}
352358
size_t will_send = (can_send < left) ? can_send : left;
353-
bool did_write = false;
354-
while( will_send ) {
359+
DEBUGV(":wr %d %d %d\r\n", will_send, left, _written);
360+
bool need_output = false;
361+
while( will_send && _datasource) {
355362
size_t next_chunk =
356363
will_send > _write_chunk_size ? _write_chunk_size : will_send;
357364
const uint8_t* buf = _datasource->get_buffer(next_chunk);
365+
if (state() == CLOSED) {
366+
need_output = false;
367+
break;
368+
}
358369
err_t err = tcp_write(_pcb, buf, next_chunk, TCP_WRITE_FLAG_COPY);
370+
DEBUGV(":wrc %d %d %d\r\n", next_chunk, will_send, err);
359371
_datasource->release_buffer(buf, next_chunk);
360372
if (err == ERR_OK) {
361373
_written += next_chunk;
362-
did_write = true;
374+
need_output = true;
375+
} else {
376+
break;
363377
}
364378
will_send -= next_chunk;
365379
}
366-
if( did_write ) tcp_output(_pcb);
367-
368-
if (!_datasource->available() || _noblock) {
369-
delete _datasource;
370-
_datasource = nullptr;
380+
if( need_output ) {
381+
tcp_output(_pcb);
371382
}
372383
}
373384

374385
void _write_some_from_cb()
375386
{
376-
_write_some();
377-
if (!_datasource && _send_waiting) {
387+
if (_send_waiting == 1) {
388+
_send_waiting--;
378389
esp_schedule();
379390
}
380391
}
@@ -490,7 +501,7 @@ class ClientContext
490501
size_t _written = 0;
491502
size_t _write_chunk_size = 256;
492503
bool _noblock = false;
493-
bool _send_waiting = false;
504+
int _send_waiting = 0;
494505
};
495506

496507
#endif//CLIENTCONTEXT_H

0 commit comments

Comments
 (0)