Skip to content

Commit

Permalink
Fix for tcp_err() callbacks and other issues (#115)
Browse files Browse the repository at this point in the history
* Fixes the handling tcp_err() callback and addresses build issues
with different core releases.

Summary:

The operator "new ..." was changed to "new (std::nothrow) ...", which will
return NULL when the heap is out of memory. Without the change "soft WDT"
was the result, starting with Arduino ESP8266 Core 2.5.0. (Note, RE:"soft
WDT" - the error  reporting may improve with core 2.6.) With proir core
versions the library appears to work fine.
ref: esp8266/Arduino#6269 (comment)

To support newer lwIP versions and buffer models. All references to 1460
were replaced with TCP_MSS. If TCP_MSS is not defined (exp. 1.4v lwIP)
1460 is assumed.

The ESPAsyncTCP library should build for Arduino ESP8266 Core releases:
2.3.0, 2.4.1, 2.4.2, 2.5.1, 2.5.2. It may still build with core versions
2.4.0 and 2.5.0. I did not do any regression testing with these, since
they had too many issues and were quickly superseded.

lwIP tcp_err() callback often resulted in crashes. The problem was a
tcp_err() would come in, while processing a send or receive in the
forground. The tcp_err() callback would be passed down to a client's
registered disconnect CB. A common problem with SyncClient and other
modules as well as some client code was: the freeing of ESPAsyncTCP
AsyncClient objects via disconnect CB handlers while the library was
waiting for an operstion to  finished. Attempts to access bad pointers
followed. For SyncClient this commonly occured during a call to delay().
On return to SyncClient _client was invalid. Also the problem described by
issue #94 also surfaced

Use of tcp_abort() required some very special handling and was very
challenging to make work without changing client API. ERR_ABRT can only be
used once on a return to lwIP for a given connection and since the
AsyncClient structure was sometimes deleted before returning to lwIP, the
state tracking became tricky. While ugly, a global variable for this
seemed to work; however, I  abanded it when I saw a possible
reentrancy/concurrency issue. After several approaches I settled the
problem by creating "class ACErrorTracker" to manage the issue.

Additional Async Client considerations:

The client sketch must always test if the connection is still up at loop()
entry and after the return of any function call, that may have done a
delay() or yield() or any ESPAsyncTCP library family call. For example,
the connection could be lost during a call to _client->write(...). Client
sketches that delete _client as part of their onDisconnect() handler must
be very careful as _client will become invalid after calls to delay(),
yield(), etc.

* moved DebugPrintMacros.h to the correct loccation.
  • Loading branch information
mhightower83 authored and me-no-dev committed Sep 21, 2019
1 parent 545260d commit 75c2513
Show file tree
Hide file tree
Showing 14 changed files with 1,517 additions and 840 deletions.
4 changes: 2 additions & 2 deletions library.json
Expand Up @@ -12,11 +12,11 @@
"type": "git",
"url": "https://github.com/me-no-dev/ESPAsyncTCP.git"
},
"version": "1.2.0",
"version": "1.2.1",
"license": "LGPL-3.0",
"frameworks": "arduino",
"platforms": "espressif8266",
"build": {
"libCompatMode": 2
}
}
}
2 changes: 1 addition & 1 deletion library.properties
@@ -1,5 +1,5 @@
name=ESP AsyncTCP
version=1.2.0
version=1.2.1
author=Me-No-Dev
maintainer=Me-No-Dev
sentence=Async TCP Library for ESP8266 and ESP31B
Expand Down
55 changes: 41 additions & 14 deletions src/AsyncPrinter.cpp
Expand Up @@ -28,7 +28,7 @@ AsyncPrinter::AsyncPrinter()
, _close_cb(NULL)
, _close_arg(NULL)
, _tx_buffer(NULL)
, _tx_buffer_size(1460)
, _tx_buffer_size(TCP_MSS)
, next(NULL)
{}

Expand All @@ -43,7 +43,10 @@ AsyncPrinter::AsyncPrinter(AsyncClient *client, size_t txBufLen)
, next(NULL)
{
_attachCallbacks();
_tx_buffer = new cbuf(_tx_buffer_size);
_tx_buffer = new (std::nothrow) cbuf(_tx_buffer_size);
if(_tx_buffer == NULL) {
panic(); //What should we do?
}
}

AsyncPrinter::~AsyncPrinter(){
Expand All @@ -63,10 +66,14 @@ void AsyncPrinter::onClose(ApCloseHandler cb, void *arg){
int AsyncPrinter::connect(IPAddress ip, uint16_t port){
if(_client != NULL && connected())
return 0;
_client = new AsyncClient();
_client = new (std::nothrow) AsyncClient();
if (_client == NULL) {
panic();
}

_client->onConnect([](void *obj, AsyncClient *c){ ((AsyncPrinter*)(obj))->_onConnect(c); }, this);
if(_client->connect(ip, port)){
while(_client->state() < 4)
while(_client && _client->state() < 4)
delay(1);
return connected();
}
Expand All @@ -76,23 +83,32 @@ int AsyncPrinter::connect(IPAddress ip, uint16_t port){
int AsyncPrinter::connect(const char *host, uint16_t port){
if(_client != NULL && connected())
return 0;
_client = new AsyncClient();
_client = new (std::nothrow) AsyncClient();
if (_client == NULL) {
panic();
}

_client->onConnect([](void *obj, AsyncClient *c){ ((AsyncPrinter*)(obj))->_onConnect(c); }, this);
if(_client->connect(host, port)){
while(_client->state() < 4)
while(_client && _client->state() < 4)
delay(1);
return connected();
}
return 0;
}

void AsyncPrinter::_onConnect(AsyncClient *c){
(void)c;
if(_tx_buffer != NULL){
cbuf *b = _tx_buffer;
_tx_buffer = NULL;
delete b;
}
_tx_buffer = new cbuf(_tx_buffer_size);
_tx_buffer = new (std::nothrow) cbuf(_tx_buffer_size);
if(_tx_buffer) {
panic();
}

_attachCallbacks();
}

Expand All @@ -109,7 +125,11 @@ AsyncPrinter & AsyncPrinter::operator=(const AsyncPrinter &other){
_tx_buffer = NULL;
delete b;
}
_tx_buffer = new cbuf(other._tx_buffer_size);
_tx_buffer = new (std::nothrow) cbuf(other._tx_buffer_size);
if(_tx_buffer == NULL) {
panic();
}

_client = other._client;
_attachCallbacks();
return *this;
Expand All @@ -127,13 +147,16 @@ size_t AsyncPrinter::write(const uint8_t *data, size_t len){
while(_tx_buffer->room() < toSend){
toWrite = _tx_buffer->room();
_tx_buffer->write((const char*)data, toWrite);
while(!_client->canSend())
while(connected() && !_client->canSend())
delay(0);
if(!connected())
return 0; // or len - toSend;
_sendBuffer();
toSend -= toWrite;
}
_tx_buffer->write((const char*)(data+(len - toSend)), toSend);
while(!_client->canSend()) delay(0);
while(connected() && !_client->canSend()) delay(0);
if(!connected()) return 0; // or len - toSend;
_sendBuffer();
return len;
}
Expand All @@ -154,7 +177,11 @@ size_t AsyncPrinter::_sendBuffer(){
size_t sendable = _client->space();
if(sendable < available)
available= sendable;
char *out = new char[available];
char *out = new (std::nothrow) char[available];
if (out == NULL) {
panic(); // Connection should be aborted instead
}

_tx_buffer->read(out, available);
size_t sent = _client->write(out, available);
delete out;
Expand All @@ -180,8 +207,8 @@ void AsyncPrinter::_on_close(){
}

void AsyncPrinter::_attachCallbacks(){
_client->onPoll([](void *obj, AsyncClient* c){ ((AsyncPrinter*)(obj))->_sendBuffer(); }, this);
_client->onAck([](void *obj, AsyncClient* c, size_t len, uint32_t time){ ((AsyncPrinter*)(obj))->_sendBuffer(); }, this);
_client->onPoll([](void *obj, AsyncClient* c){ (void)c; ((AsyncPrinter*)(obj))->_sendBuffer(); }, this);
_client->onAck([](void *obj, AsyncClient* c, size_t len, uint32_t time){ (void)c; (void)len; (void)time; ((AsyncPrinter*)(obj))->_sendBuffer(); }, this);
_client->onDisconnect([](void *obj, AsyncClient* c){ ((AsyncPrinter*)(obj))->_on_close(); delete c; }, this);
_client->onData([](void *obj, AsyncClient* c, void *data, size_t len){ ((AsyncPrinter*)(obj))->_onData(data, len); }, this);
_client->onData([](void *obj, AsyncClient* c, void *data, size_t len){ (void)c; ((AsyncPrinter*)(obj))->_onData(data, len); }, this);
}
2 changes: 1 addition & 1 deletion src/AsyncPrinter.h
Expand Up @@ -46,7 +46,7 @@ class AsyncPrinter: public Print {
AsyncPrinter *next;

AsyncPrinter();
AsyncPrinter(AsyncClient *client, size_t txBufLen = 1460);
AsyncPrinter(AsyncClient *client, size_t txBufLen = TCP_MSS);
virtual ~AsyncPrinter();

int connect(IPAddress ip, uint16_t port);
Expand Down
96 changes: 96 additions & 0 deletions src/DebugPrintMacros.h
@@ -0,0 +1,96 @@
#ifndef _DEBUG_PRINT_MACROS_H
#define _DEBUG_PRINT_MACROS_H
// Some customizable print macros to suite the debug needs de jour.

// Debug macros
// #include <pgmspace.h>
// https://stackoverflow.com/questions/8487986/file-macro-shows-full-path
// This value is resolved at compile time.
#define _FILENAME_ strrchr("/" __FILE__, '/')

// #define DEBUG_ESP_ASYNC_TCP 1
// #define DEBUG_ESP_TCP_SSL 1
// #define DEBUG_ESP_PORT Serial

#if defined(DEBUG_ESP_PORT) && !defined(DEBUG_TIME_STAMP_FMT)
#define DEBUG_TIME_STAMP_FMT "%06u.%03u "
struct _DEBUG_TIME_STAMP {
unsigned dec;
unsigned whole;
};
inline struct _DEBUG_TIME_STAMP debugTimeStamp(void) {
struct _DEBUG_TIME_STAMP st;
unsigned now = millis() % 1000000000;
st.dec = now % 1000;
st.whole = now / 1000;
return st;
}
#endif

#if defined(DEBUG_ESP_PORT) && !defined(DEBUG_GENERIC)
#define DEBUG_GENERIC( module, format, ... ) \
do { \
struct _DEBUG_TIME_STAMP st = debugTimeStamp(); \
DEBUG_ESP_PORT.printf( DEBUG_TIME_STAMP_FMT module " " format, st.whole, st.dec, ##__VA_ARGS__ ); \
} while(false)
#endif
#if defined(DEBUG_ESP_PORT) && !defined(DEBUG_GENERIC_P)
#define DEBUG_GENERIC_P( module, format, ... ) \
do { \
struct _DEBUG_TIME_STAMP st = debugTimeStamp(); \
DEBUG_ESP_PORT.printf_P(PSTR( DEBUG_TIME_STAMP_FMT module " " format ), st.whole, st.dec, ##__VA_ARGS__ ); \
} while(false)
#endif

#if defined(DEBUG_GENERIC) && !defined(ASSERT_GENERIC)
#define ASSERT_GENERIC( a, module ) \
do { \
if ( !(a) ) { \
DEBUG_GENERIC( module, "%s:%s:%u: ASSERT("#a") failed!\n", __FILE__, __func__, __LINE__); \
DEBUG_ESP_PORT.flush(); \
} \
} while(false)
#endif
#if defined(DEBUG_GENERIC_P) && !defined(ASSERT_GENERIC_P)
#define ASSERT_GENERIC_P( a, module ) \
do { \
if ( !(a) ) { \
DEBUG_GENERIC_P( module, "%s:%s:%u: ASSERT("#a") failed!\n", __FILE__, __func__, __LINE__); \
DEBUG_ESP_PORT.flush(); \
} \
} while(false)
#endif

#ifndef DEBUG_GENERIC
#define DEBUG_GENERIC(...) do { (void)0;} while(false)
#endif

#ifndef DEBUG_GENERIC_P
#define DEBUG_GENERIC_P(...) do { (void)0;} while(false)
#endif

#ifndef ASSERT_GENERIC
#define ASSERT_GENERIC(...) do { (void)0;} while(false)
#endif

#ifndef ASSERT_GENERIC_P
#define ASSERT_GENERIC_P(...) do { (void)0;} while(false)
#endif

#ifndef DEBUG_ESP_PRINTF
#define DEBUG_ESP_PRINTF( format, ...) DEBUG_GENERIC_P("[%s]", format, &_FILENAME_[1], ##__VA_ARGS__)
#endif

#if defined(DEBUG_ESP_ASYNC_TCP) && !defined(ASYNC_TCP_DEBUG)
#define ASYNC_TCP_DEBUG( format, ...) DEBUG_GENERIC_P("[ASYNC_TCP]", format, ##__VA_ARGS__)
#endif

#ifndef ASYNC_TCP_ASSERT
#define ASYNC_TCP_ASSERT( a ) ASSERT_GENERIC_P( (a), "[ASYNC_TCP]")
#endif

#if defined(DEBUG_ESP_TCP_SSL) && !defined(TCP_SSL_DEBUG)
#define TCP_SSL_DEBUG( format, ...) DEBUG_GENERIC_P("[TCP_SSL]", format, ##__VA_ARGS__)
#endif

#endif //_DEBUG_PRINT_MACROS_H

0 comments on commit 75c2513

Please sign in to comment.