Skip to content

Commit

Permalink
Dispose task listeners and emmitters when needed
Browse files Browse the repository at this point in the history
Signed-off-by: elaihau <liang.huang@ericsson.com>
  • Loading branch information
elaihau committed Jul 3, 2019
1 parent b10d2fe commit fb1e1cb
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 30 deletions.
9 changes: 5 additions & 4 deletions packages/task/src/node/process/process-task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,11 @@ export class ProcessTask extends Task {
) {
super(taskManager, logger, options);

const toDispose = this.process.onExit(async event => {
toDispose.dispose();
this.fireTaskExited(await this.getTaskExitedEvent(event));
});
this.toDispose.push(
this.process.onExit(async event => {
this.fireTaskExited(await this.getTaskExitedEvent(event));
})
);

// Buffer to accumulate incoming output.
let databuf: string = '';
Expand Down
55 changes: 31 additions & 24 deletions packages/task/src/node/task-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
********************************************************************************/

import { inject, injectable, named } from 'inversify';
import { ILogger } from '@theia/core/lib/common/';
import { Disposable, DisposableCollection, ILogger } from '@theia/core/lib/common/';
import {
TaskClient,
TaskExitedEvent,
Expand All @@ -32,10 +32,11 @@ import { ProcessTask } from './process/process-task';
import { ProblemCollector } from './task-problem-collector';

@injectable()
export class TaskServerImpl implements TaskServer {
export class TaskServerImpl implements TaskServer, Disposable {

/** Task clients, to send notifications-to. */
protected clients: TaskClient[] = [];
protected toDispose = new DisposableCollection();

@inject(ILogger) @named('task')
protected readonly logger: ILogger;
Expand All @@ -50,7 +51,7 @@ export class TaskServerImpl implements TaskServer {
private problemCollectors: Map<string, Map<number, ProblemCollector>> = new Map();

dispose() {
// do nothing
this.toDispose.dispose();
}

async getTasks(context?: string): Promise<TaskInfo[]> {
Expand All @@ -70,31 +71,37 @@ export class TaskServerImpl implements TaskServer {
async run(taskConfiguration: TaskConfiguration, ctx?: string, option?: RunTaskOption): Promise<TaskInfo> {
const runner = this.runnerRegistry.getRunner(taskConfiguration.type);
const task = await runner.run(taskConfiguration, ctx);
this.toDispose.push(task);

task.onExit(event => {
this.taskManager.delete(task);
this.fireTaskExitedEvent(event);
this.removedCachedProblemCollector(event.ctx || '', event.taskId);
});
this.toDispose.push(
task.onExit(event => {
this.taskManager.delete(task);
this.fireTaskExitedEvent(event);
this.removedCachedProblemCollector(event.ctx || '', event.taskId);
this.toDispose.dispose();
})
);

const resolvedMatchers = option && option.customization ? option.customization.problemMatcher || [] : [];
if (resolvedMatchers.length > 0) {
task.onOutput(event => {
let collector: ProblemCollector | undefined = this.getCachedProblemCollector(event.ctx || '', event.taskId);
if (!collector) {
collector = new ProblemCollector(resolvedMatchers);
this.cacheProblemCollector(event.ctx || '', event.taskId, collector);
}

const problems = collector.processLine(event.line);
if (problems.length > 0) {
this.fireTaskOutputProcessedEvent({
taskId: event.taskId,
ctx: event.ctx,
problems
});
}
});
this.toDispose.push(
task.onOutput(event => {
let collector: ProblemCollector | undefined = this.getCachedProblemCollector(event.ctx || '', event.taskId);
if (!collector) {
collector = new ProblemCollector(resolvedMatchers);
this.cacheProblemCollector(event.ctx || '', event.taskId, collector);
}

const problems = collector.processLine(event.line);
if (problems.length > 0) {
this.fireTaskOutputProcessedEvent({
taskId: event.taskId,
ctx: event.ctx,
problems
});
}
})
);
}

const taskInfo = await task.getRuntimeInfo();
Expand Down
11 changes: 9 additions & 2 deletions packages/task/src/node/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
********************************************************************************/

import { injectable } from 'inversify';
import { ILogger, Emitter, Event, MaybePromise } from '@theia/core/lib/common/';
import { ILogger, Disposable, DisposableCollection, Emitter, Event, MaybePromise } from '@theia/core/lib/common/';
import { TaskManager } from './task-manager';
import { TaskInfo, TaskExitedEvent, TaskConfiguration, TaskOutputEvent, TaskOutputProcessedEvent } from '../common/task-protocol';

Expand All @@ -26,9 +26,10 @@ export interface TaskOptions {
}

@injectable()
export abstract class Task {
export abstract class Task implements Disposable {

protected taskId: number;
protected toDispose: DisposableCollection = new DisposableCollection();

This comment has been minimized.

Copy link
@akosyakov

akosyakov Jul 3, 2019

Member

readonly?

This comment has been minimized.

Copy link
@elaihau

elaihau Jul 3, 2019

Contributor

sure i will add readonly to both toDispose i added

readonly exitEmitter: Emitter<TaskExitedEvent>;
readonly outputEmitter: Emitter<TaskOutputEvent>;
readonly outputProcessedEmitter: Emitter<TaskOutputProcessedEvent>;
Expand All @@ -41,6 +42,8 @@ export abstract class Task {
this.taskId = this.taskManager.register(this, this.options.context);
this.exitEmitter = new Emitter<TaskExitedEvent>();
this.outputEmitter = new Emitter<TaskOutputEvent>();
this.toDispose.push(this.exitEmitter);
this.toDispose.push(this.outputEmitter);

This comment has been minimized.

Copy link
@akosyakov

akosyakov Jul 3, 2019

Member

what about outputProcessedEmitter?

This comment has been minimized.

Copy link
@elaihau

elaihau Jul 3, 2019

Contributor

oops outputProcessedEmitter is not referenced. it will be removed.

}

/** Terminates the task. */
Expand Down Expand Up @@ -77,4 +80,8 @@ export abstract class Task {
get label() {
return this.options.label;
}

dispose() {
this.toDispose.dispose();
}
}

0 comments on commit fb1e1cb

Please sign in to comment.