Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: tons of memory leak #2754

Merged
merged 3 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/esbuild.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const ctx = await esbuild[args.watch ? 'context' : 'build']({
color: true,
loader: { '.svg': 'file', '.ttf': 'file' },
sourcemap: args.watch,
minify: !args.watch,
minify: false,
target: 'chrome70',
plugins: [
copyPlugin({
Expand Down
24 changes: 12 additions & 12 deletions examples/src/sheets/lazy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,22 @@
import type { Plugin, PluginCtor } from '@univerjs/core';
import { UniverSheetsFilterUIPlugin } from '@univerjs/sheets-filter-ui';
import { UniverSheetsFindReplacePlugin } from '@univerjs/sheets-find-replace';
import { UniverUniscriptPlugin } from '@univerjs/uniscript';
// import { UniverUniscriptPlugin } from '@univerjs/uniscript';

export default function getLazyPlugins(): Array<[PluginCtor<Plugin>] | [PluginCtor<Plugin>, unknown]> {
return [
[
UniverUniscriptPlugin,
{
getWorkerUrl(moduleID: string, label: string) {
if (label === 'typescript' || label === 'javascript') {
return './vs/language/typescript/ts.worker.js';
}
// [
// UniverUniscriptPlugin,
// {
// getWorkerUrl(moduleID: string, label: string) {
// if (label === 'typescript' || label === 'javascript') {
// return './vs/language/typescript/ts.worker.js';
// }

return './vs/editor/editor.worker.js';
},
},
],
// return './vs/editor/editor.worker.js';
// },
// },
// ],
[UniverSheetsFilterUIPlugin],
[UniverSheetsFindReplacePlugin],
];
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/common/request-immediate-macro-task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@ export function requestImmediateMacroTask(callback: (value?: unknown) => void):
const channel = new MessageChannel();
let cancelled = false;

channel.port1.onmessage = () => {
const hanlder = () => {
if (!cancelled) {
callback();
}
};

// This would cause memory leak. But we cannot use addEventListener because it won't work in web worker.
channel.port1.onmessage = hanlder;
channel.port2.postMessage(null);

return () => {
cancelled = true;
// dispose
channel.port1.onmessage = null;
channel.port1.close();
channel.port2.close();
};
Expand Down
13 changes: 11 additions & 2 deletions packages/core/src/services/command/command.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { createIdentifier, Inject, Injector } from '@wendellhu/redi';

import { findLast, remove } from '../../common/array';
import { sequence, sequenceAsync } from '../../common/sequence';
import { DisposableCollection, toDisposable } from '../../shared/lifecycle';
import { Disposable, DisposableCollection, toDisposable } from '../../shared/lifecycle';
import type { IKeyValue } from '../../shared/types';
import { IContextService } from '../context/context.service';
import { ILogService } from '../log/log.service';
Expand Down Expand Up @@ -215,7 +215,7 @@ export const NilCommand: ICommand = {
handler: () => true,
};

export class CommandService implements ICommandService {
export class CommandService extends Disposable implements ICommandService {
protected readonly _commandRegistry: CommandRegistry;

private readonly _beforeCommandExecutionListeners: CommandListener[] = [];
Expand All @@ -231,10 +231,19 @@ export class CommandService implements ICommandService {
@Inject(Injector) private readonly _injector: Injector,
@ILogService private readonly _logService: ILogService
) {
super();

this._commandRegistry = new CommandRegistry();
this._registerCommand(NilCommand);
}

override dispose(): void {
super.dispose();

this._commandExecutedListeners.length = 0;
this._beforeCommandExecutionListeners.length = 0;
}

hasCommand(commandId: string): boolean {
return this._commandRegistry.hasCommand(commandId);
}
Expand Down
2 changes: 2 additions & 0 deletions packages/debugger/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
"@univerjs/core": "workspace:*",
"@univerjs/design": "workspace:*",
"@univerjs/engine-render": "workspace:*",
"@univerjs/facade": "workspace:*",
"@univerjs/sheets": "workspace:*",
"@univerjs/sheets-drawing-ui": "workspace:*",
"@univerjs/ui": "workspace:*",
Expand All @@ -71,6 +72,7 @@
"devDependencies": {
"@univerjs/core": "workspace:*",
"@univerjs/engine-render": "workspace:*",
"@univerjs/facade": "workspace:*",
"@univerjs/shared": "workspace:*",
"@univerjs/sheets": "workspace:*",
"@univerjs/sheets-drawing-ui": "workspace:*",
Expand Down
4 changes: 4 additions & 0 deletions packages/debugger/src/commands/commands/unit.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@
*/

import { CommandType, type ICommand, IUniverInstanceService, type Univer, UniverInstanceType } from '@univerjs/core';
import type { FUniver } from '@univerjs/facade';

declare global {
// eslint-disable-next-line ts/naming-convention
interface Window {
univer?: Univer;
univerAPI?: ReturnType<typeof FUniver.newAPI>;
}
}

Expand All @@ -28,6 +30,8 @@ export const DisposeUniverCommand: ICommand = {
type: CommandType.COMMAND,
handler: () => {
window.univer?.dispose();
window.univer = undefined;
window.univerAPI = undefined;

return true;
},
Expand Down
11 changes: 9 additions & 2 deletions packages/debugger/src/controllers/e2e/e2e-memory.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { ICommandService, IUniverInstanceService, LifecycleStages, LocaleType, OnLifecycle, UniverInstanceType } from '@univerjs/core';
import { Disposable, ICommandService, IUniverInstanceService, LifecycleStages, LocaleType, OnLifecycle, UniverInstanceType } from '@univerjs/core';
import { DisposeUniverCommand } from '../../commands/commands/unit.command';

const DEFAULT_WORKBOOK_DATA_DEMO = {
Expand Down Expand Up @@ -80,14 +80,21 @@ declare global {
* This controller expose a API on `Window` for the E2E memory test.
*/
@OnLifecycle(LifecycleStages.Starting, E2EMemoryController)
export class E2EMemoryController {
export class E2EMemoryController extends Disposable {
constructor(
@IUniverInstanceService private readonly _univerInstanceService: IUniverInstanceService,
@ICommandService private readonly _commandService: ICommandService
) {
super();

this._initPlugin();
}

override dispose(): void {
// @ts-ignore
window.E2EControllerAPI = undefined;
}

private _initPlugin(): void {
window.E2EControllerAPI = {
loadAndRelease: (id) => this._releaseAndLoad(id),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ export class TextSelectionRenderManager extends RxDisposable implements ITextSel
super.dispose();

this._detachEvent();

this._removeAllTextRanges();
this._container.remove();
}

Expand Down
3 changes: 3 additions & 0 deletions packages/engine-render/src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,9 @@ export class Engine extends ThinEngine<Scene> {

this._beginFrame$.complete();
this._endFrame$.complete();

this._resizeObserver?.disconnect();
this._container = null;
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/engine-render/src/layer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ export class Layer extends Disposable {
});
this.clear();

this._debounceDirtyFunc?.();
this._debounceDirtyFunc = null;

this._cacheCanvas?.dispose();
Expand Down
8 changes: 7 additions & 1 deletion packages/rpc/src/services/rpc/channel.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import type { IDisposable } from '@wendellhu/redi';
import { createIdentifier } from '@wendellhu/redi';

import type { IChannel, IMessageProtocol } from './rpc.service';
Expand All @@ -29,7 +30,7 @@ export const IRPCChannelService = createIdentifier<IRPCChannelService>('IRPCChan
/**
* This service is responsible for managing the RPC channels.
*/
export class ChannelService {
export class ChannelService implements IDisposable {
private readonly _client: ChannelClient;
private readonly _server: ChannelServer;

Expand All @@ -38,6 +39,11 @@ export class ChannelService {
this._server = new ChannelServer(_messageProtocol);
}

dispose(): void {
this._client.dispose();
this._server.dispose();
}

requestChannel(name: string): IChannel {
return this._client.getChannel(name);
}
Expand Down
14 changes: 12 additions & 2 deletions packages/rpc/src/services/rpc/rpc.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import { RxDisposable } from '@univerjs/core';
import type { Subscriber, Subscription } from 'rxjs';
import type { Subscription } from 'rxjs';
import { BehaviorSubject, firstValueFrom, isObservable, Observable, of } from 'rxjs';
import { filter, take, takeUntil } from 'rxjs/operators';

Expand Down Expand Up @@ -175,7 +175,6 @@ export class ChannelClient extends RxDisposable implements IChannelClient {
private _initialized = new BehaviorSubject<boolean>(false);
private _lastRequestCounter = 0;
private _pendingRequests = new Map<number, IResponseHandler>();
private _pendingSubscriptions = new Map<number, Subscriber<any>>();

constructor(private readonly _protocol: IMessageProtocol) {
super();
Expand All @@ -185,6 +184,10 @@ export class ChannelClient extends RxDisposable implements IChannelClient {
this._protocol.onMessage.pipe(takeUntil(this.dispose$)).subscribe((message) => this._onMessage(message));
}

override dispose(): void {
this._pendingRequests.clear();
}

getChannel<T extends IChannel>(channelName: string): T {
const self = this;

Expand Down Expand Up @@ -326,6 +329,13 @@ export class ChannelServer extends RxDisposable implements IChannelServer {
this._sendResponse({ seq: -1, type: ResponseType.INITIALIZE });
}

override dispose(): void {
super.dispose();

this._subscriptions.clear();
this._channels.clear();
}

registerChannel(channelName: string, channel: IChannel): void {
this._channels.set(channelName, channel);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ export class ConditionalFormattingFormulaService extends Disposable {
}));
}

override dispose(): void {
this._formulaChange$.complete();
}

private _initCache() {
const _conditionalFormattingService = this._injector.get(ConditionalFormattingService);
this.disposeWithMe(_conditionalFormattingService.ruleComputeStatus$.subscribe((option) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export class DataValidationController extends RxDisposable {

private _initInstanceChange() {
const disposableCollection = new DisposableCollection();
this._univerInstanceService.getCurrentTypeOfUnit$<Workbook>(UniverInstanceType.UNIVER_SHEET).subscribe((workbook) => {
this.disposeWithMe(this._univerInstanceService.getCurrentTypeOfUnit$<Workbook>(UniverInstanceType.UNIVER_SHEET).subscribe((workbook) => {
disposableCollection.dispose();
if (!workbook) {
return;
Expand All @@ -104,7 +104,7 @@ export class DataValidationController extends RxDisposable {
}
})
));
});
}));

this.disposeWithMe(disposableCollection);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ export class RegisterOtherFormulaService extends Disposable {
this._initFormulaCalculationResultChange();
}

override dispose(): void {
super.dispose();

this._formulaChange$.complete();
this._formulaResult$.complete();
}

private _ensureCacheMap(unitId: string, subUnitId: string) {
let unitMap = this._formulaCacheMap.get(unitId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,8 @@ export class BaseSelectionRenderService extends Disposable implements ISheetSele
protected readonly _renderManagerService: IRenderManagerService
) {
super();

this._resetStyle();
// @ts-expect-error
if (!window.srs) window.srs = this;
}

protected _setStyle(style: ISelectionStyle) {
Expand Down
3 changes: 0 additions & 3 deletions packages/ui/src/common/component-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,13 +292,10 @@ export class ComponentManager {
async function renderVue3Component(VueComponent: ReturnType<typeof defineComponent>, element: HTMLElement, args: Record<string, any>) {
try {
const { h, render } = await import('vue');

const vnode = h(VueComponent, args);

const container = document.createElement('div');

document.body.appendChild(container);

render(vnode, element);
} catch (error) {
}
Expand Down
6 changes: 4 additions & 2 deletions packages/ui/src/controllers/ui/ui-desktop.controller.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,15 @@ function bootstrap(
render();

return toDisposable(() => {
unmount(mountContainer);
// https://github.com/facebook/react/issues/26031
createRoot(<div></div>, mountContainer);
setTimeout(() => createRoot(<div></div>, mountContainer), 200);
setTimeout(() => unmount(mountContainer), 500);
});
}

function createContainer(id: string): HTMLElement {
const element = document.createElement('div');
element.id = id;
// FIXME: the element is not append to the DOM tree. So it won't be rendered.
return element;
}
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading