Skip to content

Commit

Permalink
fix(core): cleanup memory variables leaking
Browse files Browse the repository at this point in the history
  • Loading branch information
blakebyrnes committed Jan 24, 2023
1 parent ec218b3 commit e3ae146
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 42 deletions.
50 changes: 50 additions & 0 deletions .github/workflows/js-branch.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
name: 'Publish a built Javascript Branch'

on:
push:
branches:
workflow_dispatch:

jobs:
build:
name: Build Javascript
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3

- uses: actions/setup-node@v3
with:
node-version: 18
cache: yarn

- name: Clone ulixee/shared
run: git clone https://github.com/ulixee/shared.git
working-directory: ../..

- name: Install ulixee/shared
run: yarn build
working-directory: ../../shared

- name: Clone unblocked
run: git clone --recurse-submodules -j8 https://github.com/ulixee/unblocked.git
working-directory: ../..

- name: Install unblocked
run: yarn build
working-directory: ../../unblocked

- name: Build modules
run: yarn && yarn build:dist --network-timeout 1000000

- name: Publish branch
run: |
cd build-dist
git config --global user.email "staff@ulixee.org"
git config --global user.name "CI"
git init -b main
git add -A
git commit -m 'Auto-build Javascript files'
git push -f https://ulixee:${{ env.GH_TOKEN }}@github.com/ulixee/hero.git main:${{ github.ref_name }}-built-js
shell: bash
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
2 changes: 1 addition & 1 deletion core/dbs/SessionDb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ export default class SessionDb {
}

SessionDb.byId.delete(this.sessionId);
this.db.close();
if (deleteFile) {
this.db.close();
await Fs.promises.rm(this.path);
}
this.db = null;
Expand Down
17 changes: 11 additions & 6 deletions core/lib/CommandRecorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ type AsyncFunc = (...args: any[]) => Promise<any>;

export default class CommandRecorder {
public readonly fnNames = new Set<string>();
private readonly fnMap = new Map<string, AsyncFunc>();
private logger: IBoundLog;
private isClosed = false;

Expand All @@ -20,8 +21,10 @@ export default class CommandRecorder {
fns: AsyncFunc[],
) {
for (const fn of fns) {
// used for bypassing recording
owner[`___${fn.name}`] = fn.bind(owner);
owner[fn.name] = ((...args) => this.runCommandFn(fn, ...args)) as any;
this.fnMap.set(fn.name, fn.bind(owner));
owner[fn.name] = this.runCommandFn.bind(this, fn.name);
this.fnNames.add(fn.name);
}
this.logger = log.createChild(module, {
Expand All @@ -35,12 +38,14 @@ export default class CommandRecorder {
this.isClosed = true;
this.session = null;
this.owner = null;
this.fnMap.clear();
}

private async runCommandFn<T>(commandFn: AsyncFunc, ...args: any[]): Promise<T> {
private async runCommandFn<T>(functionName: string, ...args: any[]): Promise<T> {
if (this.isClosed) return;
if (!this.fnNames.has(commandFn.name))
throw new Error(`Unsupported function requested ${commandFn.name}`);
const commandFn = this.fnMap.get(functionName);
if (!this.fnNames.has(functionName) || !commandFn)
throw new Error(`Unsupported function requested ${functionName}`);

const { session, owner } = this;
if (session === null) return;
Expand All @@ -50,7 +55,7 @@ export default class CommandRecorder {
session.commands.presetMeta = null;

const shouldWait =
!owner.shouldWaitForCommandLock || owner.shouldWaitForCommandLock(commandFn.name);
!owner.shouldWaitForCommandLock || owner.shouldWaitForCommandLock(functionName);
if (shouldWait) await commands.waitForCommandLock();

let tabId = this.tabId;
Expand All @@ -68,7 +73,7 @@ export default class CommandRecorder {
tabId,
frameId,
frame?.navigations?.top?.id,
commandFn.name,
functionName,
args,
meta,
);
Expand Down
16 changes: 13 additions & 3 deletions core/lib/Session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export default class Session

public readonly id: string;
public readonly baseDir: string;
public readonly plugins: CorePlugins;
public plugins: CorePlugins;

public get browserEngine(): IBrowserEngine {
return this.emulationProfile.browserEngine;
Expand Down Expand Up @@ -171,7 +171,7 @@ export default class Session
'rerun-kept-alive': void;
}>();

public readonly agent: Agent;
public agent: Agent;

protected readonly logger: IBoundLog;

Expand All @@ -195,7 +195,6 @@ export default class Session
this.id = this.getId(options.sessionId);
const id = this.id;
Session.byId[id] = this;
this.events.once(this, 'closed', () => delete Session.byId[id]);
this.db = new SessionDb(this.id);
this.commands = new Commands(this.db);

Expand Down Expand Up @@ -469,6 +468,7 @@ export default class Session
}

public getLastActiveTab(): Tab {
if (!this.commands) return null;
for (let idx = this.commands.history.length - 1; idx >= 0; idx -= 1) {
const command = this.commands.history[idx];
if (command.tabId) {
Expand Down Expand Up @@ -562,6 +562,7 @@ export default class Session
this.emit('closed', closedEvent);
await closedEvent.waitForPromise;

delete Session.byId[this.id];
this.events.close();
this.commandRecorder.cleanup();
this.plugins.cleanup();
Expand All @@ -571,6 +572,7 @@ export default class Session
LogEvents.unsubscribe(this.logSubscriptionId);
loggerSessionIdNames.delete(this.id);
this.db.flush();
this.cleanup();

this.removeAllListeners();
try {
Expand Down Expand Up @@ -660,6 +662,14 @@ export default class Session
return sessionId ?? nanoid();
}

private cleanup(): void {
this.agent = null;
this.commandRecorder = null;
this.browserContext = null;
this.plugins = null;
this.commands = null;
}

private onResource(event: BrowserContext['resources']['EventTypes']['change']): void {
this.db.resources.insert(
event.tabId,
Expand Down
18 changes: 8 additions & 10 deletions core/lib/Tab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export default class Tab
}

public readonly parentTabId?: number;
public readonly session: Session;
public session: Session;
public readonly frameEnvironmentsById = new Map<number, FrameEnvironment>();
public readonly frameEnvironmentsByDevtoolsId = new Map<string, FrameEnvironment>();
public page: Page;
Expand Down Expand Up @@ -247,9 +247,7 @@ export default class Tab
await this.page.setJavaScriptEnabled(enableJs);
}

public setBlockedResourceUrls(
blockedUrls: (string | RegExp)[],
): void {
public setBlockedResourceUrls(blockedUrls: (string | RegExp)[]): void {
const mitmSession = this.session.mitmRequestSession;

let interceptor = mitmSession.interceptorHandlers.find(x => x.types && !x.handlerFn);
Expand Down Expand Up @@ -308,10 +306,14 @@ export default class Tab
}
}
this.events.close();
this.commandRecorder.cleanup();
this.commandRecorder = null;
this.emit('close');
// clean up listener memory
this.removeAllListeners();
this.session = null;
this.frameEnvironmentsById.clear();
this.frameEnvironmentsByDevtoolsId.clear();

this.logger.stats('Tab.Closed', { parentLogId, errors });
}
Expand Down Expand Up @@ -841,14 +843,10 @@ export default class Tab
private async waitForReady(): Promise<void> {
await this.mainFrameEnvironment.isReady;
if (this.session.options?.blockedResourceTypes) {
await this.setBlockedResourceTypes(
this.session.options.blockedResourceTypes,
);
await this.setBlockedResourceTypes(this.session.options.blockedResourceTypes);
}
if (this.session.options?.blockedResourceUrls) {
await this.setBlockedResourceUrls(
this.session.options.blockedResourceUrls,
);
await this.setBlockedResourceUrls(this.session.options.blockedResourceUrls);
}
}

Expand Down
2 changes: 1 addition & 1 deletion core/test/user-profile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -523,9 +523,9 @@ document.querySelector('#local').innerHTML = localStorage.getItem('local');
'textContent',
]);
expect(crossContent.value).toBe('1');
const history = tab.navigations.history;
await session.close();

const history = tab.navigations.history;
expect(history).toHaveLength(1);
expect(history[0].finalUrl).toBe(`${koaServer.baseUrl}/cross-storage2`);
}
Expand Down
21 changes: 16 additions & 5 deletions timetravel/lib/DomStateGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import IResourceFilterProperties from '@ulixee/hero-interfaces/IResourceFilterPr
import { IStorageChangesEntry } from '@ulixee/hero-core/models/StorageChangesTable';
import Resolvable from '@ulixee/commons/lib/Resolvable';
import BrowserContext from '@ulixee/unblocked-agent/lib/BrowserContext';
import Session from '@ulixee/hero-core/lib/Session';
import { NodeType } from './DomNode';
import DomRebuilder from './DomRebuilder';
import MirrorPage from './MirrorPage';
Expand Down Expand Up @@ -48,7 +49,10 @@ export default class DomStateGenerator {
sessionId,
needsProcessing: true,
mainFrameIds: sessionDb.frames.mainFrameIds(),
db: sessionDb,
// could get closed, so need to use getter
get db(): SessionDb {
return SessionDb.getCached(sessionId, false);
},
dbLocation: SessionDb.databaseDir,
loadingRange: [...loadingRange],
timelineRange: timelineRange ? [...timelineRange] : undefined,
Expand Down Expand Up @@ -83,15 +87,18 @@ export default class DomStateGenerator {
};
}
for (const session of savedState.sessions) {
const sessionId = session.sessionId;
let db: SessionDb;
try {
db = SessionDb.getCached(session.sessionId, false);
db = SessionDb.getCached(sessionId, false);
} catch (err) {
// couldn't load
}
this.sessionsById.set(session.sessionId, {
this.sessionsById.set(sessionId, {
...session,
db,
get db(): SessionDb {
return SessionDb.getCached(sessionId, false);
},
needsProcessing: !!db,
mainFrameIds: db?.frames.mainFrameIds(session.tabId),
});
Expand Down Expand Up @@ -188,7 +195,11 @@ export default class DomStateGenerator {

// wait for end point
if (timeoutMs > 0) await new Promise(resolve => setTimeout(resolve, timeoutMs));
if (!db.readonly) db.flush();

const liveSession = Session.get(sessionId);
if (liveSession && liveSession.db?.isOpen && !liveSession.db?.readonly) {
liveSession.db.flush();
}

session.mainFrameIds = db.frames.mainFrameIds(tabId);

Expand Down
24 changes: 8 additions & 16 deletions timetravel/lib/MirrorPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,12 @@ export default class MirrorPage extends TypedEventEmitter<{
super();
this.setDomRecording(domRecording);
this.onPageEvents = this.onPageEvents.bind(this);
this.logger = log.createChild(module);
}

public async attachToPage(page: Page, sessionId: string, setReady = true): Promise<void> {
this.page = page;
this.logger = log.createChild(module, { sessionId });
let readyResolvable: Resolvable<void>;
if (setReady) {
readyResolvable = new Resolvable<void>();
Expand All @@ -77,16 +79,10 @@ export default class MirrorPage extends TypedEventEmitter<{
this.events.once(page, 'close', this.close.bind(this));
if (this.debugLogging) {
this.events.on(page, 'console', msg => {
log.info('MirrorPage.console', {
...msg,
sessionId,
});
this.logger.info('MirrorPage.console', msg);
});
this.events.on(page, 'crashed', msg => {
log.info('MirrorPage.crashed', {
...msg,
sessionId,
});
this.logger.info('MirrorPage.crashed', msg);
});
}

Expand Down Expand Up @@ -123,6 +119,7 @@ export default class MirrorPage extends TypedEventEmitter<{
if (this.isReady) return await this.isReady;

this.sessionId = sessionId;
this.logger = log.createChild(module, { sessionId });
const ready = new Resolvable<void>();
this.isReady = ready.promise;
try {
Expand Down Expand Up @@ -209,10 +206,9 @@ export default class MirrorPage extends TypedEventEmitter<{
isLoadingDocument = true;

if (this.debugLogging) {
log.info('MirrorPage.navigate', {
this.logger.info('MirrorPage.navigate', {
newPaintIndex,
url: loadingDocument.url,
sessionId: this.sessionId,
});
}
const loader = await this.page.navigate(loadingDocument.url);
Expand All @@ -229,9 +225,8 @@ export default class MirrorPage extends TypedEventEmitter<{

if (newPaintIndex >= 0) {
if (this.debugLogging) {
log.info('MirrorPage.loadPaintEvents', {
this.logger.info('MirrorPage.loadPaintEvents', {
newPaintIndex,
sessionId: this.sessionId,
});
}

Expand Down Expand Up @@ -272,8 +267,6 @@ export default class MirrorPage extends TypedEventEmitter<{
}

public async close(): Promise<void> {
if (this.isReady === null) return;
this.isReady = null;
this.loadQueue.stop();
if (this.page && !this.page.isClosed) {
await this.page.close();
Expand Down Expand Up @@ -337,9 +330,8 @@ export default class MirrorPage extends TypedEventEmitter<{
return frame;
}
} catch (error) {
log.warn('Error matching frame nodeId to environment', {
this.logger.warn('Error matching frame nodeId to environment', {
error,
sessionId: this.sessionId,
});
// just keep looking?
}
Expand Down

0 comments on commit e3ae146

Please sign in to comment.