ckaczor/azuredatastudio
1
0
Fork 0
mirror of https://github.com/ckaczor/azuredatastudio.git synced 2026-02-16 18:46:40 -05:00
Code Issues Packages Projects Releases Wiki Activity

Notebooks: More Jupyter Server Hardening (#5264)

Browse Source 
* Do not rely on same starting port every time, misc

* put back to 5 seconds for process kill timeout

* extHostNotebook shutdown manager handle
This commit is contained in:
Chris LaFreniere
2019-04-30 14:13:04 -07:00
committed by GitHub
parent 0f12d15020
commit e72d0d03ed
4 changed files with 13 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
extensions/notebook/src/common/ports.ts
View File

@@ -7,7 +7,7 @@
import * as net from 'net'; import * as net from 'net';
export class StrictPortFindOptions { export class StrictPortFindOptions {
constructor(public startPort: number, public minPort: number, public maxport: number) { constructor(public minPort: number, public maxport: number) {
} }
public maxRetriesPerStartPort: number = 5; public maxRetriesPerStartPort: number = 5;
public totalRetryLoops: number = 10; public totalRetryLoops: number = 10;
@@ -21,7 +21,7 @@ export class StrictPortFindOptions {
*/ */
export async function strictFindFreePort(options: StrictPortFindOptions): Promise<number> { export async function strictFindFreePort(options: StrictPortFindOptions): Promise<number> {
let totalRetries = options.totalRetryLoops; let totalRetries = options.totalRetryLoops;
let startPort = options.startPort; let startPort = getRandomInt(options.minPort, options.maxport);
let port = await findFreePort(startPort, options.maxRetriesPerStartPort, options.timeout); let port = await findFreePort(startPort, options.maxRetriesPerStartPort, options.timeout);
while (port === 0 && totalRetries > 0) { while (port === 0 && totalRetries > 0) {
startPort = getRandomInt(options.minPort, options.maxport); startPort = getRandomInt(options.minPort, options.maxport);

9
extensions/notebook/src/jupyter/serverInstance.ts
View File

@@ -86,8 +86,9 @@ export class ServerInstanceUtils {
try { try {
process.kill(childProcess.pid, 'SIGKILL'); process.kill(childProcess.pid, 'SIGKILL');
} catch (error) { } catch (error) {
console.log(error); if (!error || !error.code || (typeof error.code === 'string' && error.code !== 'ESRCH')) {
// All is fine. console.log(error);
}
} }
}, 5000); }, 5000);
} }
@@ -230,8 +231,8 @@ export class PerNotebookServerInstance implements IServerInstance {
return; return;
} }
let notebookDirectory = this.getNotebookDirectory(); let notebookDirectory = this.getNotebookDirectory();
// Find a port in a given range. If run into trouble, got up 100 in range and search inside a larger range // Find a port in a given range. If run into trouble, try another port inside the given range
let port = await ports.strictFindFreePort(new ports.StrictPortFindOptions(defaultPort, defaultPort + 100, defaultPort + 1000)); let port = await ports.strictFindFreePort(new ports.StrictPortFindOptions(defaultPort, defaultPort + 1000));
let token = await notebookUtils.getRandomToken(); let token = await notebookUtils.getRandomToken();
this._uri = vscode.Uri.parse(`http://localhost:${port}/?token=${token}`); this._uri = vscode.Uri.parse(`http://localhost:${port}/?token=${token}`);
this._port = port.toString(); this._port = port.toString();

4
extensions/notebook/src/test/common/port.test.ts
View File

@@ -39,7 +39,7 @@ describe('Ports', () => {
this.timeout(1000 * 10); // higher timeout for this test this.timeout(1000 * 10); // higher timeout for this test
// get an initial freeport >= 7000 // get an initial freeport >= 7000
let options = new ports.StrictPortFindOptions(7000, 7100, 7200); let options = new ports.StrictPortFindOptions(7000, 7200);
options.timeout = 300000; options.timeout = 300000;
ports.strictFindFreePort(options).then(initialPort => { ports.strictFindFreePort(options).then(initialPort => {
assert.ok(initialPort >= 7000); assert.ok(initialPort >= 7000);
@@ -49,7 +49,7 @@ describe('Ports', () => {
server.listen(initialPort, undefined, undefined, () => { server.listen(initialPort, undefined, undefined, () => {
// once listening, find another free port and assert that the port is different from the opened one // once listening, find another free port and assert that the port is different from the opened one
options.startPort = initialPort; options.minPort = initialPort;
options.maxRetriesPerStartPort = 1; options.maxRetriesPerStartPort = 1;
options.totalRetryLoops = 50; options.totalRetryLoops = 50;
ports.strictFindFreePort(options).then(freePort => { ports.strictFindFreePort(options).then(freePort => {

4
src/sql/workbench/api/node/extHostNotebook.ts
View File

@@ -117,6 +117,10 @@ export class ExtHostNotebook implements ExtHostNotebookShape {
} }
$shutdownSession(managerHandle: number, sessionId: string): Thenable<void> { $shutdownSession(managerHandle: number, sessionId: string): Thenable<void> {
// If manager handle has already been removed, don't try to access it again when shutting down
if (this._adapters.get(managerHandle) === undefined) {
return undefined;
}
return this._withSessionManager(managerHandle, async (sessionManager) => { return this._withSessionManager(managerHandle, async (sessionManager) => {
return sessionManager.shutdown(sessionId); return sessionManager.shutdown(sessionId);
}); });