Occasional infinite loop in previously working code path in 6.5+

[tgriesser]

• Version: 6.5+

This is a strange issue I'm having trouble reproducing myself or creating a minimum viable test for, but recently in some CI suites in Node 6.5+ there have been infinite loops occurring in code paths that previously had no issue.

It seems that they're occurring when a method (dynamically named on the prototype) calls another dynamically named method. Instead of calling the correct method, it appears to be instead calling itself resulting in an infinite loop.

For example:

const modifiers = [
  'default', 'defaultsTo', 'defaultTo', 'unsigned',
  'nullable', 'notNull', 'notNullable'
];
each(modifiers, function(method) {
  ColumnBuilder.prototype[method] = function() {
    if (method === 'notNullable') return this.nullable(false);
    this._modifiers[method] = toArray(arguments);
    return this;
  };
});

It seems that at a certain point when the method builder.notNullable() is called instead of then calling/returning this.nullable(false) it instead calls itself again and ends up in an infinite loop.

Seeing as the v8 version was bumped in 6.5 I suspect this might be a bug there, but I'm not sure where to begin researching if that might be the case.

/cc @jamesdixon @fl0w @kirrg001 @ErisDS who reported seeing this issue, as they might be able to provide more info on the platforms they're seeing this on and provide better insight into the problem.

Here's the original issue as it was reported: knex/knex#1725

These are the two code snippets in the codebase where this has been seen to occur:
https://github.com/tgriesser/knex/blob/82685b57f0f61b053a02899941c78ced3e04163d/src/schema/tablebuilder.js#L156-L182
https://github.com/tgriesser/knex/blob/a24e4df6380bfda9b6e2ef05368ac23a0a2ccaab/src/schema/columnbuilder.js#L30-L39

[MylesBorins]

@tgriesser where is that each function you are using above coming from? Do you have the same issue when using modifiers.forEach?

[tgriesser]

It's coming from lodash. Haven't tried with forEach, and actually haven't even been able to reproduce this locally myself yet, once I'm able to I'll give it a shot.

[evanlucas]

/cc @nodejs/v8

I tried to reproduce this yesterday and was unable. @tgriesser have you been able to come up with a reproducible test case for this?

[fl0w]

I'm trying to reproduce this as well, I've re-run our tests a while now and can't produce this again - though we haven't really done any upgrades.

node v6.7
knex 0.12.0 (with which this originally occurred)

[tgriesser]

Nope, I haven't been able to reproduce this so I'm closing. I've since split out the offending code path so folks using knex shouldn't see this issue anymore.

This ticket was opened mainly because issue had been reported by a few different users at the same time, so I figured there was a chance it was indicative of a bigger issue.