Skip to content

Instantly share code, notes, and snippets.

@deepak1556
Created June 21, 2021 16:42
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save deepak1556/1a5f694b5361a71de33a34ceba38deab to your computer and use it in GitHub Desktop.
Save deepak1556/1a5f694b5361a71de33a34ceba38deab to your computer and use it in GitHub Desktop.
OOP shared worker
<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<title>Hello World!</title>
</head>
<body>
<h1>Hello World!</h1>
</body>
<script>
// https://developer.mozilla.org/en-US/docs/Web/API/SharedWorker
var myWorker = new SharedWorker("worker.js");
// Message ports allow IPC between the OOP worker and this document
myWorker.port.onmessage = function(e) {
console.log('Message received from worker');
console.log(e.data)
}
myWorker.port.postMessage('start');
</script>
</html>
// Modules to control application life and create native browser window
const {app, BrowserWindow, protocol} = require('electron')
const path = require('path')
protocol.registerSchemesAsPrivileged([
{
scheme: 'foo',
privileges: {
standard: true,
secure: true
}
}
])
function createWindow () {
protocol.registerFileProtocol('foo', (request, cb) => {
const url = new URL(request.url)
console.log(`Loading : ${__dirname + url.pathname}`)
if (url.pathname === "/worker.js") {
cb({
path: __dirname + url.pathname
})
} else {
// The headers below force the shared worker to be Out-Of-Process
cb({
path: __dirname + url.pathname,
headers: {
"Cross-Origin-Opener-Policy": "same-origin",
"Cross-Origin-Embedder-Policy": "require-corp"
}
})
}
})
// Create the browser window.
const mainWindow = new BrowserWindow({
width: 800,
height: 600,
webPreferences: {
}
})
// and load the index.html of the app.
mainWindow.loadURL('foo://custom/index.html')
// Open the DevTools.
// mainWindow.webContents.openDevTools()
}
// This method will be called when Electron has finished
// initialization and is ready to create browser windows.
// Some APIs can only be used after this event occurs.
app.whenReady().then(() => {
createWindow()
app.on('activate', function () {
// On macOS it's common to re-create a window in the app when the
// dock icon is clicked and there are no other windows open.
if (BrowserWindow.getAllWindows().length === 0) createWindow()
})
})
// Quit when all windows are closed, except on macOS. There, it's common
// for applications and their menu bar to stay active until the user quits
// explicitly with Cmd + Q.
app.on('window-all-closed', function () {
if (process.platform !== 'darwin') app.quit()
})
// In this file you can include the rest of your app's specific main process
// code. You can also put them in separate files and require them here.
onconnect = function(e) {
var port = e.ports[0];
port.onmessage = function(e) {
port.postMessage('worker connected');
}
}
@deepak1556
Copy link
Author

deepak1556 commented Jun 22, 2021

Thanks for the feedback!

So for example, even today IIRC we have seen some crashes because we run electron.exe with ELECTRON_RUN_AS_NODE to simulate node.exe, namely IIRC in some cryptography stuff, in some i18n stuff, with some env variables, etc.

Agree, this arises from the fact electron had to share the following libraries from chromium with node to integrate both together. But since this done at build time, the libraries are shared even if we spawn a pure node child process from electron. So behavior won't be different with node integrated shared worker and a node child process in terms of the api surface.

  • BoringSSL the cryptographic library from chromium replaces the openSSL version used by node
  • V8 engine
  • I18n library

As you have mentioned, the cases we don't know that can break in this model can only be found through experimentation down the line.

with some env variables, etc.

This issue is eliminated with the shared worker process as the way chromium creates the process is slightly different from what we do with the child process module.

My other concern is that enabling node integration in SharedWorker might defeat the purpose of sandboxing the renderer process; an attacker could now simply create a SharedWorker and then gain spawning privileges on the system; I fear it might invalidate the sandboxing effort of the renderer process.

This is a great point, when adding the node capability to shared workers I plan to expose an event on the app module based on https://source.chromium.org/chromium/chromium/src/+/main:content/public/browser/content_browser_client.h;l=682-690 that will get called whenever a shared worker is requested to be created with relevant details about the requestor document as well as the worker endpoint. This allows app to finely control whether they want to allow node integration in the worker or not and also whether to start the worker. Does this address the security concern ?

app.on('will-create-shared-worker', (event, details) => {
  /**
   * event Object
     * preventDefault Function
     * enableNodeIntegration Boolean
   **/ 
  /**
   * details Object
     * workerURL String
     * documentOrigin String
     * document webContents
   **/
});

I suggest we continue as proposed

Agree with your plan, just to add some pointers to 3)

see if we have any MessagePort from Electron, and if not, we can look into using a WebSocket to localhost from the sandboxed renderer.

This not currently worked on in the Electron land, and I did start my exploration from here. Originally, I looked to establish message ports between a node child process and a chromium renderer process. Digging at the internals, the message ports are closely tied to the event loop mechanism of chromium, so it required binding some chromium libraries in the node child process which would eventually end up as a chromium process with node integration enabled. From there, I landed on this out-of-process shared worker which was already performing most of what is required.

In conclusion, is there a general interest to pursue this model further for vscode ? Could we experiment with migrating the file watcher service or any other candidates that are less complex to extension host ? which can give us data points to drive future conversations about this model ?

@bpasero
Copy link

bpasero commented Jun 23, 2021

I like the idea of exploring shared workers with node.js integration for file services specifically because I do not think this service benefits from running in the background from the shared process. It still needs to be isolated process wise given file watch events can be expensive. One thing that would be nice to have is if the shared worker could also have a direct communication channel to the extension host to deliver file events, but that is maybe P2.

My main worry for file service adoption is how expensive / slow it is to create such a shared worker. The file service is obviously one of those things we need right from the beginning because many things get resolved on startup. We can try to offload some of that work into the initial config object we send over but it might become unpractical very quickly.

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