apigee-127 / swagger-node-runner

The heart of Swagger-Node
MIT License
102 stars 123 forks source link

Memory leak #110

Closed tbflw closed 6 years ago

tbflw commented 6 years ago

After spending a lot of time investigating, debugging and heap dumping a problem across all our swagger-node production services we have localised a memory leak in swagger-node-runer 0.7.3. The leak is most likely caused by a downstream dependency (z-schema) which was released in version 3.18.3 on August 19th. This release contains a new implementation of a ReferenceCache which we suspect is the cause of the leak (https://github.com/zaggino/z-schema/commit/b1ea164b7acba124de45d149b42279411e6eefeb).

Here's a screenshot showing the memory usage of one of our services with the memory leak and without.

service_memory_leak_2017-09-11_20 35 26

The screenshot without the memory leak is from the service built with the following shrinkwrap configuration to lock down the dependency on z-schema to version 3.18.2:

{
  "dependencies": {
    "swagger-express-mw": {
      "version": "0.7.0",
      "from": "swagger-express-mw@0.7.0",
      "dependencies": {
        "swagger-node-runner": {
          "version": "0.7.3",
          "from": "swagger-node-runner@0.7.3",
          "dependencies": {
            "sway": {
              "version": "1.0.0",
              "from": "sway@1.0.0",
              "dependencies": {
                "z-schema": {
                  "version": "3.18.2",
                  "from": "z-schema@3.18.2"
                }
              }
            }
          }
        }
      }
    }
  }
}
theganyo commented 6 years ago

Thank you for tracking this down! Since this is a Sway dependency issue, do you mind opening an issue there and linking back to this?

theganyo commented 6 years ago

Actually, it should really be addressed by z-schema. It would be best to go to the source and get it fixed instead of attempting to lock it down to a prior version.

tbflw commented 6 years ago

Since this bug is handled upstream, I think it's OK to close it.