These are chat archives for SmingHub/Sming

14th
Nov 2017
bbourdel
@bbourdel
Nov 14 2017 11:22

@slaff Thanks !!

I've found the issue that cause HardwareWatchdog timer.
millis() is an unsigned long (uint32_t) that overflow at 2^32/1000 mS !
Because, millis() is defined as system_get_time()/1000000UL and system_get_time() is a micro-second counter that over at 2^32uS.
In my code, I compare an uint32_t timestamp with millis(). As millis() overflow before my timestamp, my code gets stuck in a while loop -> watchdog reset.

It can be a good idea to solve or warn people about this. @slaff, should we add a note in readme file about it ?
Arduino toolkit for ESP8266 manage millis() in a good way (with an overflow after 2^32mS).

slaff
@slaff
Nov 14 2017 12:07
@bbourdel We can add a note in the doc comment for the millis function with an example. Or we can port Arduino's millis to Sming.
slaff
@slaff
Nov 14 2017 12:15
For the moment the first option will be the easiest solution. Or if you want you can port the millis code from Arduino to Sming.
Érico GR
@ericogr
Nov 14 2017 12:42
@slaff I got memory leak using mqtt with ssl enabled. It occurs when broker restart (using mqtt.connect(...) again to reconnect)
In TcpConnection I add ssl_ctx_free(sslContext) when closing and onError. Apparently worked but I'm not shure if it can cause side effects
Érico GR
@ericogr
Nov 14 2017 12:55
if you prefer, I can submit a PR to be analysed
slaff
@slaff
Nov 14 2017 13:03
PR should be fine. ssl_ctx_free(sslContext) should be called inside of ssl_close(ssl).
bbourdel
@bbourdel
Nov 14 2017 14:45
@slaff Got arduino millis() code : https://github.com/esp8266/Arduino/blob/master/cores/esp8266/core_esp8266_wiring.c
It's basicly a SoftTimer that manage micro overflow every minute.
@ericogr I have the same memory leaks every time a Http Client is closed by the online server (in SSL). About 400bytes each time
bbourdel
@bbourdel
Nov 14 2017 14:56
@slaff, I've seen you've push the developp branch with this comment in HttpClient.cpp :
  // DON'T call cleanup. 
  // If you want to free all resources from HttpClients the correct sequence will be to 
  // 1. Delete all instances of HttpClient 
  // 2. Call the static method HttpClient::cleanup();
I'm not sure we "delete all instances of HttpClient" ?
Is it a todo list for Sming Core developpement or an info for app developper ?
bbourdel
@bbourdel
Nov 14 2017 15:52
I'm not sure how to "delete all instances of HttpClient" ?
Érico GR
@ericogr
Nov 14 2017 16:08
#1288 @slaff @bbourdel This is a try to fix this memory leak... apparently, it worked with for my context but I'm not a C/low level programming :worried:
slaff
@slaff
Nov 14 2017 16:21

Is it a todo list for Sming Core developpement or an info for app developper ?

It is warning to a SmingCore developer not to put there the cleanup and description why.

@ericogr Your PR is getting in the right direction. Please, take a look at my comments, apply the requested changes and test again on your device.
Érico GR
@ericogr
Nov 14 2017 16:48
@slaff worked in the same way
I'm shure there are others memory leaks in reconnection, but now it spend less memory
slaff
@slaff
Nov 14 2017 16:49
Ok, so is the issue that you are experiencing fixed?
Érico GR
@ericogr
Nov 14 2017 16:50
yes...
slaff
@slaff
Nov 14 2017 16:52
Ok, good. I will check it later this week with valgrind. If you find other memory leaks - just create a PR and we will look over it.
Érico GR
@ericogr
Nov 14 2017 16:52
ok, thanks for your help
slaff
@slaff
Nov 14 2017 16:54

I just saw that there is one other place where you should make a change - in TcpConnection::staticOnConnected there is a code:

if(con->ssl != NULL) {
    ssl_free(con->ssl);
}

it should be replaced with

if(con->sslContext != NULL) {
    ssl_ctx_free(con->sslContext);
}

ok, thanks for your help

Thanks for your help too :)

Érico GR
@ericogr
Nov 14 2017 19:01
ok @slaff , done