Skip to content

Commit

Permalink
Plugin API: Added module unpatching and updated tests correspondingly (…
Browse files Browse the repository at this point in the history 
…#383)

PR-URL: #383
  • Loading branch information
@kjin
kjin authored Feb 14, 2017
1 parent 9011c8a commit 9743556
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 59 deletions.
41 changes: 26 additions & 15 deletions src/trace-plugin-loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,7 @@ function activate(agent) {
// hook into Module._load so that we can hook into userspace frameworks
shimmer.wrap(Module, '_load', function(originalModuleLoad) {
function loadAndPatch(instrumentation, moduleRoot, version) {
if (!instrumentation.patches[moduleRoot]) {
instrumentation.patches[moduleRoot] = {};
}
var patchSet = instrumentation.patches[moduleRoot][version];
var patchSet = instrumentation.patches[moduleRoot];
if (!patchSet) {
// Load the plugin object
var plugin = originalModuleLoad(instrumentation.file, module, false);
Expand All @@ -87,17 +84,18 @@ function activate(agent) {
patchSet[file] = {
file: file,
patch: patch.patch,
unpatch: patch.unpatch,
intercept: patch.intercept
};
}
});
if (Object.keys(patchSet).length === 0) {
logger.warn(moduleRoot + ': version ' + version + ' not supported ' +
logger.warn(moduleRoot + ': version ' + version + ' not supported ' +
'by plugin.');
}
instrumentation.patches[moduleRoot][version] = patchSet;
instrumentation.patches[moduleRoot] = patchSet;
}
Object.keys(patchSet).forEach(function(file) {
for (var file in patchSet) {
var patch = patchSet[file];
if (!patch.module) {
var loadPath = moduleRoot ? path.join(moduleRoot, patch.file) : patch.file;
Expand All @@ -109,7 +107,7 @@ function activate(agent) {
if (patch.intercept) {
patch.module = patch.intercept(patch.module, api);
}
});
}
var rootPatch = patchSet[''];
if (rootPatch && rootPatch.intercept) {
return rootPatch.module;
Expand All @@ -119,14 +117,12 @@ function activate(agent) {
}

function moduleAlreadyPatched(instrumentation, moduleRoot, version) {
return instrumentation.patches[moduleRoot] &&
instrumentation.patches[moduleRoot][version];
return instrumentation.patches[moduleRoot];
}

// Future requires get patched as they get loaded.
return function Module_load(request, parent, isMain) {
var instrumentation = plugins[request];

if (instrumentation) {
var moduleRoot = util.findModulePath(request, parent);
var moduleVersion = util.findModuleVersion(moduleRoot, originalModuleLoad);
Expand All @@ -140,17 +136,32 @@ function activate(agent) {
return patchedRoot;
}
}

return originalModuleLoad.apply(this, arguments);
};
});
}

function deactivate() {
activated = false;
if (activated) {
activated = false;
for (var moduleName in plugins) {
var instrumentation = plugins[moduleName];
for (var moduleRoot in instrumentation.patches) {
var patchSet = instrumentation.patches[moduleRoot];
for (var file in patchSet) {
var patch = patchSet[file];
if (patch.unpatch !== undefined) {
logger.info('Unpatching' + moduleName);
patch.unpatch(patch.module);
}
}
}
}
plugins = {};

// unhook module.load
shimmer.unwrap(Module, '_load');
// unhook module.load
shimmer.unwrap(Module, '_load');
}
}

module.exports = {
Expand Down
82 changes: 38 additions & 44 deletions test/test-plugin-loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,52 +252,12 @@ describe('Trace Plugin Loader', function() {
'Files internal to a module are patched');
});

/**
* Alternately loads two versions of the same module, and checks that each one
* is patched differently.
*/
it('can patch multiple different versions of the same module', function() {
var v1 = { getVersion: function() { return '1.0.0'; } };
var v2 = { getVersion: function() { return '2.0.0'; } };
addModuleMock('module-h', '1.0.0', v1);
addModuleMock('module-h-plugin', '', [
{
versions: '1.x',
patch: function(originalModule, api) {
shimmer.wrap(originalModule, 'getVersion', function(origGetVersion) {
return function() {
return origGetVersion() + ' is ok';
};
});
}
},
{
versions: '2.x',
patch: function(originalModule, api) {
shimmer.wrap(originalModule, 'getVersion', function(origGetVersion) {
return function() {
return origGetVersion() + ' is better';
};
});
}
}
]);
// Activate plugin loader
pluginLoader.activate(createFakeAgent({
'module-h': 'module-h-plugin'
}));
assert.strictEqual(require('module-h').getVersion(), '1.0.0 is ok',
'Initial patch is correct');
addModuleMock('module-h', '2.0.0', v2);
assert.strictEqual(require('module-h').getVersion(), '2.0.0 is better',
'Second loaded version is also patched');
addModuleMock('module-h', '1.0.0', v1);
assert.strictEqual(require('module-h').getVersion(), '1.0.0 is ok',
'First loaded version doesn\'t get patched again');
});
// TODO(kjin): Add test to check that two modules with different versions
// can be loaded at the same time. Current test harness is insufficient for
// doing this.

/**
* Uses module interception to
* Uses module interception to completely replace a module export
*/
it('can intercept modules', function() {
addModuleMock('module-i', '1.0.0', function() { return 1; });
Expand All @@ -314,4 +274,38 @@ describe('Trace Plugin Loader', function() {
assert.strictEqual(require('module-i')(), 2,
'Module can be intercepted');
});

/**
* Patches a module, then immediately unpatches it, then patches it again to
* show that patching isn't irreversible (and neither is unpatching)
*/
it('can unpatch', function() {
// Unfortunately, intercepted modules cannot be patched.
addModuleMock('module-j', '1.0.0', {
getPatchMode: function() { return 'none'; }
});
addModuleMock('module-j-plugin', '', [{
patch: function(originalModule, api) {
shimmer.wrap(originalModule, 'getPatchMode', function() {
return function() { return 'patch'; };
});
},
unpatch: function(originalModule) {
shimmer.unwrap(originalModule, 'getPatchMode');
}
}]);
assert.strictEqual(require('module-j').getPatchMode(), 'none');
pluginLoader.activate(createFakeAgent({
'module-j': 'module-j-plugin'
}));
assert.strictEqual(require('module-j').getPatchMode(), 'patch');
pluginLoader.deactivate();
assert.strictEqual(require('module-j').getPatchMode(), 'none',
'Module gets unpatched');
pluginLoader.activate(createFakeAgent({
'module-j': 'module-j-plugin'
}));
assert.strictEqual(require('module-j').getPatchMode(), 'patch',
'Patches still work after unpatching');
});
});

0 comments on commit 9743556

Please sign in to comment.