Skip to content
This repository has been archived by the owner on May 16, 2024. It is now read-only.

There may be a memory leak when processing memory blocks containing non-ASCII characters. #46

Open
johnlanni opened this issue Feb 1, 2024 · 4 comments

Comments

@johnlanni
Copy link
Contributor

When I implemented the envoy proxy-wasm plugin, I found that if the response body is processed by gzip compression, a memory leak will occur. This problem also exists in the coraza-waf plugin. I suspect it may be affected by non-ASCII characters. You can use these test server program to reproduce it:

ASCII Response:

package main

import (
        "fmt"
        "log"
        "math/rand"
        "net/http"
        "time"
)

func echoHandler(w http.ResponseWriter, r *http.Request) {
        maxSize := 100 * 1024              // 100KB
        randSize := rand.Intn(maxSize) + 1 

        characters := "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789 ,.!?;:"

        rand.Seed(time.Now().UnixNano())

        randomText := make([]byte, randSize)
        for i := range randomText {
                randomText[i] = characters[rand.Intn(len(characters))]
        }

        w.Header().Set("Content-Type", "text/plain")

        _, err := w.Write(randomText)
        if err != nil {
                fmt.Println("Error writing body")
                return
        }
}

func main() {
        http.HandleFunc("/", echoHandler)

        fmt.Println("Server is running on port 5000...")

        log.Fatal(http.ListenAndServe(":5000", nil))
}

Non ASCII Response

package main

import (
        "fmt"
        "log"
        "math/rand"
        "net/http"
)

func echoHandler(w http.ResponseWriter, r *http.Request) {
        maxSize := 100 * 1024              // 100KB
        randSize := rand.Intn(maxSize) + 1 
        randomData := make([]byte, randSize)
        _, err := rand.Read(randomData) 
        if err != nil {
                http.Error(w, "Failed to generate random data", http.StatusInternalServerError)
                return
        }

        _, err = w.Write(randomData)
        if err != nil {
                fmt.Println("Error writing body")
                return
        }

}
func main() {
        http.HandleFunc("/", echoHandler)
        fmt.Println("Server is running on port 5000...")
        log.Fatal(http.ListenAndServe(":5000", nil))
}
@anuraaga
Copy link
Contributor

Hi @johnlanni - very sorry for the late reply. It was tough to respond since I was hoping to come up with a better response, I think in the end it's time to be clear.

There have been various memory leaks reported and fixed, but at a fundamental level without closer integration of Tinygo and bdwgc, I suspect creating an actually good GC will not be feasible.

Go is very close to supporting wasm exports

golang/go#65199

With that, almost any WASI binary compiled with Tinygo should be compilable with Go. For any production workload concerned with performance, it probably should switch to the official compiler. The GC is strongly tied to the compiler there and will perform much better than anything achievable with Tinygo at the tradeoff of large binaries.

I plan on archiving this repo as soon as wasmexport is available in gotip. I will leave this issue open since it is indeed an issue with this repo. But unfortunately it won't be addressed and my general suggestion for the Wasm ecosystem in Go is to pause until wasmexport is available in gotip.

@johnlanni
Copy link
Contributor Author

Hi @anuraaga, thank you for the information and the work you have done in this repo to benefit us.
We currently work around this issue by identifying whether the http request/response is suitable for printable characters.
Another thing I want to ask is, even if gotip supports wasm export, how does it support gc?

@anuraaga
Copy link
Contributor

@johnlanni I'm glad to hear that printable responses still work well. I wish I knew why 😂

When using the Go compiler with GOOS=wasip1, the runtime is the same as Go. It means it is the exact Go GC we enjoy with normal Go programs. Go has long supported single core operation so the GC is well designed for it and seems to work quite well. With wasmexport, we will need to benchmark again to see how well it performs with Envoy but I expect it to be much better than the workaround in this repo.

@johnlanni
Copy link
Contributor Author

johnlanni commented Mar 12, 2024

Based on my understanding of Go's native GC, the GC process is parallel to the execution of user code. It does not perform GC during the memory allocation process like bdwgc. Wasm does not support multi-threading. How does Go's native GC achieve this?

My thought is that even if it is supported, the performance may not be better than bdwgc, because Go's GC itself is designed to run concurrently with user code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants