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

Memory leak & maximum call stack exceeded using hasMany with scope #4470

Closed
Verdier opened this Issue Sep 11, 2015 · 14 comments

Comments

3 participants
@Verdier
Contributor

Verdier commented Sep 11, 2015

Considering the following model definitions:

var User = sequelize.define('User', {
    name: Sequelize.STRING
});

var Project = sequelize.define('Project', {
    type: Sequelize.STRING,
    title: Sequelize.STRING
});

User.addScope('defaultScope', {
    include: [Project]
}, {
    override: true
});

Project.belongsTo(User);

User.hasMany(Project, {
    scope: {
        type: 'open'
    }
});

We create a user and a project:

let user = yield User.create({
    name: 'John'
});
yield user.createProject({
    name: 'Project 1'
});

Then querying the user multiple times as following:

yield User.findById(user.id);
yield User.findById(user.id);
yield User.findById(user.id);
yield User.findById(user.id);
yield User.findById(user.id);
yield User.findById(user.id);
yield User.findById(user.id);
yield User.findById(user.id);
yield User.findById(user.id);
yield User.findById(user.id);
yield User.findById(user.id);
yield User.findById(user.id);
yield User.findById(user.id);

Result in a growing array value in query-generator.js:1913 in whereItemQuery method. Logging the array value around line 1923 after the recursive call to whereItemQuery, we obtain the following:

'((((((((((((("Projects"."type" = \'open\' AND "Projects"."type" = \'open\') AND "Projects"."type" = \'open\') AND "Projects"."type" = \'open\') AND "Projects"."type" = \'open\') AND "Projects"."type" = \'open\') AND "Projects"."type" = \'open\') AND "Projects"."type" = \'open\') AND "Projects"."type" = \'open\') AND "Projects"."type" = \'open\') AND "Projects"."type" = \'open\') AND "Projects"."type" = \'open\') AND "Projects"."type" = \'open\') AND "Projects"."type" = \'open\')',
  '"Projects"."type" = \'open\'' 

The size increase at each query.

@Verdier

This comment has been minimized.

Contributor

Verdier commented Sep 11, 2015

This result in an increasing memory usage and quickly result in a call stack exceeded.

@janmeier

This comment has been minimized.

Member

janmeier commented Sep 11, 2015

I believe we've had something similar before - are you on the latest version?

@Verdier

This comment has been minimized.

Contributor

Verdier commented Sep 11, 2015

Yes I'am using 3.8.0.

@mickhansen mickhansen added the bug label Sep 11, 2015

@Verdier

This comment has been minimized.

Contributor

Verdier commented Sep 14, 2015

I'am actually looking for the origine of the issue. But I couldn't understand how a call to the query method could depend on a previous call. Is there a non reset array somewhere? that's very strange.

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 14, 2015

Likely an options object that's not cloned before use somewhere.

@Verdier

This comment has been minimized.

Contributor

Verdier commented Sep 14, 2015

@mickhansen the bug starts at query-generator.js:947

  selectQuery: function(tableName, options, model) {
    // Enter and change at your own peril -- Mick Hansen

I've not fully localised the origine (in progress) but do you remembers the reason of the comment... ?

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 14, 2015

The select query generation codebase is just slightly complex, so just a warning in humour ;)

@Verdier

This comment has been minimized.

Contributor

Verdier commented Sep 14, 2015

Okey 😄 that's probably a good location for bug 👍 🍪

@Verdier

This comment has been minimized.

Contributor

Verdier commented Sep 14, 2015

@mickhansen the problem of history is due to Model.$injectScope which inject some reference of Model.$scope (such ase $scope.include) into the option. But somewhere after, the code modify that object (ie. something in option.include and consequently Model.$scope) which is used in the next call, modified again etc...

@Verdier

This comment has been minimized.

Contributor

Verdier commented Sep 14, 2015

Here is the $scope at the first call :
screen shot 2015-09-14 at 11 23 43
At the second :
screen shot 2015-09-14 at 11 24 27
screen shot 2015-09-14 at 11 26 23
At the third :
screen shot 2015-09-14 at 11 27 34

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 14, 2015

$injectScope should not be used anywhere in selectQuery, i believe there's another open issue about the first argument scope to $injectScope being modified without being cloned first.

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 14, 2015

Only problem i see is https://github.com/sequelize/sequelize/blob/master/lib/model.js#L2541 though which reverses the array in place, shouldn't cause this bug though.

@Verdier

This comment has been minimized.

Contributor

Verdier commented Sep 14, 2015

the most strange thing is that $scope.include[0] type change after the first query, it first contain an object with a model, and then something else, which probably define the query ?!

@Verdier

This comment has been minimized.

Contributor

Verdier commented Sep 14, 2015

It's not in selectQuery, $injectScope is called from https://github.com/sequelize/sequelize/blob/master/lib/model.js#L1299. The problem is before selectQuery.

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