Skip to content

Commit

Permalink
fix: memory leaks (#1442)
Browse files Browse the repository at this point in the history
Closes #1416

- All Service Worker services now have a destroy or close method;
- Ensure all Services' listeners, connections and connections are
properly removed on destroy;
- Removed all logic that forced the Service Worker to stay awake;
- Ping JSONRPC event no longer triggers Keep Awake logic;
- Allow service worker to be paused and resumed;
- Ensure the Service Worker is properly restarted on update;
- PopupServer instances will automatically self-destroy after being
used, as they can be re-created when needed;
- Avoid permanent interval by only starting auto-lock timer when wallet
is unlocked;
- Avoid permanent Content Script interval/looped calls by opening
connection, if closed, when needed;
- Previously if any of the services threw an exception the service
worker would stop working, now it restarts and resumes execution
- Updated Dexie to 3.2.7, which brings fixes around a DB corruption
[bug](dexie/Dexie.js#543)
- Updated Fuel SDK to 0.94.5
- Fixed error in parsing headers, in Provider's middleware, when the
selected network goes offline.
  • Loading branch information
arthurgeron authored Sep 16, 2024
1 parent e555035 commit 32abae8
Show file tree
Hide file tree
Showing 30 changed files with 757 additions and 541 deletions.
6 changes: 6 additions & 0 deletions .changeset/weak-ants-cross.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@fuel-wallet/connections": patch
"fuels-wallet": patch
---

Refactored Service Worker and Content Scripts to close running processes and listeners correctly. Fixes memory leaks.
1 change: 1 addition & 0 deletions .tool-versions
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pnpm 9.10.0
2 changes: 1 addition & 1 deletion examples/cra-dapp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"@fuels/connectors": "0.25.0",
"@fuels/react": "0.25.0",
"@tanstack/react-query": "5.28.4",
"fuels": "0.94.4",
"fuels": "0.94.5",
"react": "18.3.1",
"react-dom": "18.3.1"
},
Expand Down
5 changes: 2 additions & 3 deletions packages/app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,12 @@
"compare-versions": "6.1.0",
"cross-fetch": "4.0.0",
"dayjs": "1.11.10",
"dexie": "3.2.4",
"dexie": "3.2.7",
"dexie-observable": "4.0.1-beta.13",
"dexie-react-hooks": "1.1.6",
"events": "3.3.0",
"fake-indexeddb": "4.0.2",
"framer-motion": "10.16.4",
"fuels": "0.94.4",
"fuels": "0.94.5",
"json-edit-react": "1.13.3",
"json-rpc-2.0": "1.7.0",
"lodash.debounce": "4.0.8",
Expand Down
6 changes: 0 additions & 6 deletions packages/app/src/systems/CRX/background/actions/keepAwake.ts

This file was deleted.

69 changes: 49 additions & 20 deletions packages/app/src/systems/CRX/background/communication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,58 @@ import { BACKGROUND_SCRIPT_NAME, VAULT_SCRIPT_NAME } from '@fuel-wallet/types';

import { communicationProtocol, errorBoundary } from '../utils';

import { db } from '~/systems/Core/utils/database';
import { BackgroundService } from './services/BackgroundService';
import { CommunicationProtocol } from './services/CommunicationProtocol';
import { DatabaseEvents } from './services/DatabaseEvents';
import { VaultService } from './services/VaultService';
let backgroundService: BackgroundService | null = null;
let vaultService: VaultService | null = null;
let databaseEvents: DatabaseEvents | null = null;

function onConnectHandler(port: chrome.runtime.Port) {
// Only allow connections from the extension
// This is to prevent other extensions from connecting
// to the background script and sending messages
// This is a security measure, this should never happen as
// manifest configuration externally_connectable is set to
// only allow connections from the extension itself
if (port.sender?.id !== chrome.runtime.id) {
port.disconnect();
return;
}
if ([BACKGROUND_SCRIPT_NAME, VAULT_SCRIPT_NAME].includes(port.name)) {
communicationProtocol.addConnection(port);
}
}

function onSuspendHandler() {
// Handle cleanup before the background script is suspended
backgroundService?.stop();
vaultService?.stop();
databaseEvents?.stop();
backgroundService = null;
vaultService = null;
databaseEvents = null;
db.close();
}
function onStartupHandler() {
// Handle initialization when the background script starts up
backgroundService = BackgroundService.start(communicationProtocol);
vaultService = VaultService.start(communicationProtocol);
databaseEvents = DatabaseEvents.start(communicationProtocol);
}

function onRestartHandler() {
onSuspendHandler();
onStartupHandler();
}

errorBoundary(() => {
BackgroundService.start(communicationProtocol);
VaultService.start(communicationProtocol);
DatabaseEvents.start(communicationProtocol);
// Initialize services when the background script starts up
onStartupHandler();

chrome.runtime.onConnect.addListener((port) => {
// Only allow connections from the extension
// This is to prevent other extensions from connecting
// to the background script and sending messages
// This is a security measure, this should never happen as
// manifest configuration externally_connectable is set to
// only allow connections from the extension itself
if (port.sender?.id !== chrome.runtime.id) {
port.disconnect();
return;
}
if ([BACKGROUND_SCRIPT_NAME, VAULT_SCRIPT_NAME].includes(port.name)) {
communicationProtocol.addConnection(port);
}
});
});
chrome.runtime.onConnect.addListener(onConnectHandler);
chrome.runtime.onSuspend.addListener(onSuspendHandler);
chrome.runtime.onStartup.addListener(onStartupHandler);
chrome.runtime.onUpdateAvailable.addListener(onRestartHandler);
}, onRestartHandler);
1 change: 0 additions & 1 deletion packages/app/src/systems/CRX/background/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import './actions/autoUpdate';
import './actions/keepAwake';
import './actions/onInstall';
import './communication';
131 changes: 82 additions & 49 deletions packages/app/src/systems/CRX/background/services/BackgroundService.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { BACKGROUND_SCRIPT_NAME } from '@fuel-wallet/types';
import type { Connection } from '@fuel-wallet/types';
import type { CommunicationEventArg, Connection } from '@fuel-wallet/types';
import { CONTENT_SCRIPT_NAME, MessageTypes } from '@fuels/connectors';
import { Address } from 'fuels';
import type {
JSONRPCParams,
JSONRPCRequest,
JSONRPCServerMiddlewareNext,
SimpleJSONRPCMethod,
} from 'json-rpc-2.0';
import { JSONRPCServer } from 'json-rpc-2.0';
import { APP_VERSION } from '~/config';
Expand All @@ -27,71 +28,87 @@ type EventOrigin = {
connection?: Connection;
};

type Methods<T> = {
// biome-ignore lint/suspicious/noExplicitAny: <explanation>
[K in keyof T]: T[K] extends (...args: any[]) => any ? K : never;
}[keyof T];

export class BackgroundService {
readonly server: JSONRPCServer<EventOrigin>;
readonly communicationProtocol: CommunicationProtocol;
readonly methods: Array<Methods<Omit<BackgroundService, 'methods'>>> = [
'ping',
'version',
'isConnected',
'accounts',
'connect',
'network',
'disconnect',
'signMessage',
'sendTransaction',
'currentAccount',
'addAssets',
'assets',
'addNetwork',
'addAbi',
'getAbi',
];

constructor(communicationProtocol: CommunicationProtocol) {
// Bind methods to ensure correct `this` context
this.handleRequest = this.handleRequest.bind(this);
this.connectionMiddleware = this.connectionMiddleware.bind(this);

this.communicationProtocol = communicationProtocol;
this.server = new JSONRPCServer<EventOrigin>();
this.server.applyMiddleware(this.connectionMiddleware.bind(this));
this.server.applyMiddleware(this.connectionMiddleware);
this.setupListeners();
this.externalMethods([
this.ping,
this.version,
this.isConnected,
this.accounts,
this.connect,
this.network,
this.disconnect,
this.signMessage,
this.sendTransaction,
this.currentAccount,
this.addAssets,
this.assets,
this.addNetwork,
this.addAbi,
this.getAbi,
]);
this.addExternalMethods();
}

static start(communicationProtocol: CommunicationProtocol) {
return new BackgroundService(communicationProtocol);
}

setupListeners() {
this.communicationProtocol.on(MessageTypes.request, async (event) => {
if (event.target !== BACKGROUND_SCRIPT_NAME) return;
const origin = event.sender?.origin!;
const title = event.sender?.tab?.title!;
const favIconUrl = event.sender?.tab?.favIconUrl!;
const response = await this.server.receive(event.request, {
origin,
title,
favIconUrl,
});
if (response) {
this.communicationProtocol.postMessage({
id: event.id,
type: MessageTypes.response,
target: CONTENT_SCRIPT_NAME,
response,
});
}
private async handleRequest(
event: CommunicationEventArg<MessageTypes.request>
) {
if (event.target !== BACKGROUND_SCRIPT_NAME) return;
const origin = event.sender?.origin!;
const title = event.sender?.tab?.title!;
const favIconUrl = event.sender?.tab?.favIconUrl!;
const response = await this.server.receive(event.request, {
origin,
title,
favIconUrl,
});
if (response) {
this.communicationProtocol.postMessage({
id: event.id,
type: MessageTypes.response,
target: CONTENT_SCRIPT_NAME,
response,
});
}
}

// biome-ignore lint/suspicious/noExplicitAny: <explanation>
externalMethods(methods: Array<string | any>) {
// biome-ignore lint/complexity/noForEach: <explanation>
methods.forEach((method) => {
let methodName = method;
if (method.name) {
methodName = method.name;
}
// biome-ignore lint/suspicious/noExplicitAny: <explanation>
this.server.addMethod(methodName, this[methodName].bind(this) as any);
});
setupListeners() {
this.communicationProtocol.on(MessageTypes.request, this.handleRequest);
}

private addExternalMethods() {
for (const methodName of this.methods) {
this.server.addMethod(
methodName,
this[methodName].bind(this) as SimpleJSONRPCMethod<unknown>
);
}
}

private removeExternalMethods() {
for (const methodName of this.methods) {
this.server.removeMethod(methodName);
}
}

async requireAccounts() {
Expand Down Expand Up @@ -207,6 +224,7 @@ export class BackgroundService {
favIconUrl,
totalAccounts: shownAccounts?.length || 0,
});
popupService.destroy();
}

if (authorizedApp) {
Expand Down Expand Up @@ -260,6 +278,7 @@ export class BackgroundService {
title,
favIconUrl,
});
popupService.destroy();
return signedMessage;
}

Expand Down Expand Up @@ -293,6 +312,7 @@ export class BackgroundService {
title,
favIconUrl,
});
popupService.destroy();
return signedMessage;
}

Expand Down Expand Up @@ -341,6 +361,7 @@ export class BackgroundService {
title,
favIconUrl,
});
popupService.destroy();

return true;
}
Expand Down Expand Up @@ -377,7 +398,19 @@ export class BackgroundService {
title,
favIconUrl,
});
popupService.destroy();

return true;
}

stop() {
if (this.handleRequest) {
this.communicationProtocol.removeListener(
MessageTypes.request,
this.handleRequest
);
}
this.removeExternalMethods();
PopUpService.destroyAll();
}
}
Loading

0 comments on commit 32abae8

Please sign in to comment.