Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible memory leak in custom keywords #1879

Closed
hojat-a opened this issue Jan 18, 2022 · 6 comments
Closed

Possible memory leak in custom keywords #1879

hojat-a opened this issue Jan 18, 2022 · 6 comments

Comments

@hojat-a
Copy link

hojat-a commented Jan 18, 2022

I have a server that validates every request first. When my server has a high number of requests, heap usage goes up until I face memory leak error. This problem happens when I update AJV from 6 to 8 version. Here is a small sample of my code.

What version of Ajv are you using?

8.9.0

Does the issue happen if you use the latest version?

yes

Ajv options object

const ajv = new Ajv({
  allErrors: true,
  $data: true,
  strict: false
});

Your code

package.json dependencies

 "ajv": "^8.9.0",
    "ajv-errors": "^3.0.0",
    "ajv-formats": "^2.1.1",
    "ajv-keywords": "^5.1.0",
    "axios": "^0.24.0",
    "express": "^4.17.2"

myAjv.js

const regex = {
  username: /^[a-z][a-z0-9_.]{3,24}$/i,
  password: /^[\w,\s!@#$%&?^*_~ "'][\w,\s!@#$%&?^*_~ "'\u0600-\u06FF]/
};
const Ajv = require('ajv');
const ajv = new Ajv({
  allErrors: true,
  $data: true,
  strict: false
});
require('ajv-errors')(ajv);
require('ajv-keywords')(ajv);
require("ajv-formats")(ajv);

ajv.addKeyword({
  keyword: 'username',
  compile: function (sch, parentSchema) {
    return function validate(data) {
      if (typeof data !== 'string' || !regex.username.test(data)) {
        if (!validate.errors) {
          validate.errors = [];
        }
        validate.errors.push({
          keyword: 'username',
          params: { keyword: 'username' },
          message: ({}).hasOwnProperty.call(parentSchema, 'errorMessages') && parentSchema.errorMessages.username ||
            'invalid username!!'
        });
        return false;
      }
      return true;
    };
  },
  errors: true
});

module.exports = ajv;

myServer.js

const express = require('express');
const ajv = require('./myAjv');
const app = express();
const port = 3000;
app.use(express.json());

let schema = {
  'username': [],
  'errorMessages': {
    'username': 'invalid username'
  },
};

const fs = require('fs');
const path = __dirname + '/out1.tsv';
fs.writeFileSync(path, '\tHeapUsed\r\n', 'utf8');

app.post('/ajv', (req, res) => {
  try {
    ajv.validate(schema, req.body.data);
    fs.appendFileSync(path, `\t${Math.ceil(process.memoryUsage().heapUsed / 1024)} KB\r\n`);
    res.send('done!')
  } catch (err) {
    console.log(err);
  }
})

app.listen(port, () => {
  console.log(`Example app listening at http://localhost:${port}`)
})

myScript.js

const axios = require('axios')
const startTime = new Date().getTime();

const data = {
  'myPassword': 'shahab@redis555'
};



function run() {
  axios.post('http://localhost:3000/ajv', data)
    .then(function (response) {
      console.log(response.data);
      if (new Date().getTime() - startTime > 0.2 * 60 * 60 * 1000) {
        clearInterval(interval);
        return;
      }
    })
    .catch(function (error) {
      console.log(error);
    });
}

let interval = setInterval(run, 10)

result

HeapUsed
	13533 KB
	13814 KB
	11277 KB
	7701 KB
	7826 KB
	7950 KB
	8114 KB
	8236 KB
	8428 KB
	8546 KB
	8737 KB
	8931 KB
	9049 KB
	9239 KB
	9355 KB
	9470 KB
	9587 KB
	9779 KB
	9894 KB
	10009 KB
	10125 KB
	10315 KB
	10431 KB
	10619 KB
	10809 KB
	10995 KB
	11110 KB
	11223 KB
	11410 KB
	11525 KB
	11640 KB
	11826 KB
	12012 KB
	12126 KB
	12313 KB
	12498 KB
	12683 KB
	12872 KB
	13058 KB
	13173 KB
	13358 KB
	13542 KB
	13730 KB
	13849 KB
	13962 KB
	14080 KB
	14195 KB
	14309 KB
	14423 KB
	14536 KB
	14659 KB
	14847 KB
	15035 KB
	15220 KB
	9042 KB
	9157 KB
	9343 KB
	9527 KB
	9642 KB
	9755 KB
	9941 KB
	10056 KB
	10170 KB
	10356 KB
	10552 KB
	10737 KB
	10851 KB
	11036 KB
	11221 KB
.
.
.
        124413 KB
	124570 KB
	124727 KB
	124826 KB
	124922 KB
	125080 KB
	125237 KB
	125393 KB
	125550 KB
	125648 KB
	125744 KB
	125902 KB
	126059 KB
	126155 KB
	126252 KB

What results did you expect?

heap usage stayed more stable. I also repeat this test with ajv@6 and there was no problem(everything was normal)

Are you going to resolve the issue?

No sry!!

@behroozshafiabadi
Copy link

also for me 👍

@hojat-a @epoberezkin

@epoberezkin
Copy link
Member

why are you passing the schema in request body?

Normally, you would have schema to validate against on the server, why are you receiving it from the client?

Firstly, the client can pass incorrect schema. Secondly there are multiple security risks associated with untrusted schemas, as they are compiled to code... See https://ajv.js.org/security.html#untrusted-schemas

In any case, the crux of memory leak problem is this issue: #1413

From v8 the schema itself is used as the key, not its serialisation.

If you do need to recompile schema on every validation, which is very inefficient, you should implement your own schema caching outside of Ajv. But ideally you would compile schema once, use many times, which is not what is happening now.

This article could also be helpful: https://ajv.js.org/guide/managing-schemas.html

@hojat-a
Copy link
Author

hojat-a commented Jan 19, 2022

why are you passing the schema in request body?
Normally, you would have schema to validate against on the server, why are you receiving it from the client?

Sorry. That was my mistake. It was just a simple code (bad example) to show the problem. I keep my schema server side.

If you do need to recompile schema on every validation, which is very inefficient, you should implement your own schema caching outside of Ajv. But ideally you would compile schema once, use many times, which is not what is happening now.

Thats because I have a large collection of schemas stored on my server.

This article could also be helpful: https://ajv.js.org/guide/managing-schemas.html

Thanks for your reply. @epoberezkin

@epoberezkin
Copy link
Member

epoberezkin commented Jan 19, 2022

Thats because I have a large collection of schemas stored on my server.

So you could refer to them by schema ID, in which case Ajv itself can be used as a cache. As long as you don't pass the whole schema object (that's probably is loaded from somewhere, so it's a new object every time), it will be fine. And what happens now is that Ajv compiles it again (which is performance hit, so you are not getting any value from compilation), and it also consumes the memory - which is what you are observing. And the leak happens because Ajv keeps caching them using schema itself as a key.

What's ironic is that the change was made for performance reasons, to avoid schema serialisations and the memory that serialized schemas take - but the net effect in case of incorrect usage is exactly the opposite...

@epoberezkin
Copy link
Member

epoberezkin commented Jan 19, 2022

so your logic could be either load them all on start, or if you want to load lazily, check by ID in ajs first (with getSchema) - if it's there, use it, if not - load and add to ajv.

In general it's best to never use .validate method, and instead use .getSchema / .compile (it adds to the instance by default) or .addSchema / .getSchema (the latter returning the same validation function that is returned by .compile).

@epoberezkin
Copy link
Member

I probably should just remove .validate from readme examples and put a note in API docs to discourage its use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants