tls malloc error

Toit

#tls malloc error

Thread channel in help

JWood48 12/03/2022 12:34 PM

@erikcorry

moving the thread here

See code below, is there more cleanup i need to do to avoid the malloc error - already at the second loop I get the error:

MALLOC_FAILED error.

check_repos_test:

  while true:
    https_exec := catch --trace:
      
      // Open network
      network_interface := net.open
      client := http.Client.tls network_interface
        --root_certificates=CERTIFICATES
      
      // Set headers for auth
      headers/http.Headers := http.Headers
      headers.set "PRIVATE-TOKEN" GL_TOKEN

      // Make get request
      response := client.get "gitlab.com" GL_PATH --headers=headers
      
      // Check response
      if response.status_code != 200:
        print "FAILED_WITH $response.status_code"
      else:
        result := json.decode_stream response.body
        print "RESULT: $response.status_code :: $result[0]["status"]"
        if result[0]["status"] == "success":
          print "BUILD_STATUS: SUCCESS"
        else:
          print "BUILD_STATUS: $result[0]["status"]"

      // close network interface
      network_interface.close

    if https_exec:
      print "ERROR: $https_exec"

    sleep --ms=CALL_SERVER_TIMEOUT

JWood48 12/03/2022 12:53 PM

Interesting! Put in several "print_objects" calls before during and after to monitor object counts, and now there is no out of memory error... is print_objects doing garbage collection or somhow freeing up memory?

JWood48 12/03/2022 12:59 PM

It is enough with a single print_objects before net.open, then it works every time...

JWood48 12/03/2022 01:00 PM

having a single print_objects after network_interface.close results in out of memory error every SECOND time ??

erikcorry 12/05/2022 10:55 AM

Yes, print_objects causes a full GC.

erikcorry 12/05/2022 10:59 AM

I think you need to add client.close and I think you can move the net.open outside the loop and reuse it with no close.

erikcorry 12/05/2022 11:58 AM

It's possible that you don't have a close method on the connection in your version of the HTTP library

It's supposed to auto-close when you have read the data, but it doesn't due to a bug.

erikcorry 12/05/2022 11:58 AM

Thanks for the report!

erikcorry 12/05/2022 11:59 AM

If you insert response.body.read that should trigger the auto-close.

erikcorry 12/05/2022 11:59 AM

I'll think about how to fix this.

erikcorry 12/05/2022 12:43 PM

(Without the auto-close, it is left to the garbage collector to close the connection. That works, but it releases with a delay, so you can still get OOM.)

JWood48 12/06/2022 03:14 PM

Thanks!!

JWood48 12/06/2022 03:30 PM

just tried calling response.body.read directly, but it did not have an effect... by the way, the json.decode_stream already called the read method

JWood48 12/06/2022 03:30 PM

I dont have the client.close method

erikcorry_arbat 12/08/2022 02:43 PM

OK I'm not sure why that doesn't help you. The idea is that calling read one more time after the json deserialization makes the HTTP library realize there's nothing left and initiate auto-close.

erikcorry_arbat 12/08/2022 02:46 PM

We are working on a couple of things to help with the OOM issues. The next version of the HTTP library will be able to make multiple HTTP requests on a single TLS socket, so that you don't have to reconnect all the time. This is in the https://github.com/toitlang/pkg-http repo. You can check it out and use a local import to try it.

jag pkg uninstall http
jag pkg install --local --name=http ../../path/to/repo

GitHub - toitlang/pkg-http: Package: Implement your REST server or ...

Package: Implement your REST server or client in Toit and run it on your ESP32. - GitHub - toitlang/pkg-http: Package: Implement your REST server or client in Toit and run it on your ESP32.

erikcorry_arbat 12/08/2022 02:47 PM

The reason it is not yet rolled out is that it is not backwards compatible. You must call close on the client now, it does not auto-close. This could make the memory leaks worse if people don't call close. We will likely release it soon with a major version bump to reflect the changed API.

erikcorry_arbat 12/08/2022 02:48 PM

The other thing we are doing is moving a lot of the TLS connection logic into Toit, so that we don't have to allocate such big buffers on the C++ size. This will also mean that lost connections can be cleaned up with a single GC. The situation right now where you sometimes need more than one GC is caused by the hybrid C++/Toit solution for MbedTLS. If you are interested in seeing how the sausage is made, the code review for this new feature is at: https://github.com/toitlang/toit/pull/1263

Tls in toit by erikcorry · Pull Request #1263 · toitlang/toit

bitphlipphar 12/15/2022 02:29 PM

The initial TLS in Toit work is part of SDK v2.0.0-alpha.45 that ships in Jaguar v1.7.13.

JWood48 12/22/2022 09:32 AM

Latest version seems to have fixed this issue, thanks!

bitphlipphar 12/22/2022 09:33 AM

w00t!

21 messages in total