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

Bug 1727776 - Attempt to fix crash at submitPing on Qt in iOS #744

Closed
wants to merge 9 commits into from
Closed
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
9 changes: 7 additions & 2 deletions bin/update-glean-parser-version.sh
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,13 @@ run $SED -i.bak -E \
"${WORKSPACE_ROOT}/${FILE}"
run rm "${WORKSPACE_ROOT}/${FILE}.bak"

# Update the version in samples/qt-qml-app/requirements.txt
FILE=samples/qt-qml-app/requirements.txt
FILE=samples/qt/python/requirements.txt
run $SED -i.bak -E \
-e "s/glean_parser==[0-9.]+/glean_parser==${NEW_VERSION}/" \
"${WORKSPACE_ROOT}/${FILE}"
run rm "${WORKSPACE_ROOT}/${FILE}.bak"

FILE=samples/qt/cpp/requirements.txt
run $SED -i.bak -E \
-e "s/glean_parser==[0-9.]+/glean_parser==${NEW_VERSION}/" \
"${WORKSPACE_ROOT}/${FILE}"
Expand Down
7 changes: 3 additions & 4 deletions glean/package-lock.json

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

1 change: 1 addition & 0 deletions glean/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@
"dependencies": {
"fflate": "^0.7.1",
"jose": "^3.14.0",
"tslib": "^1.14.1",
"uuid": "^8.3.2"
},
"engines": {
Expand Down
89 changes: 65 additions & 24 deletions glean/src/core/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@ export const enum DispatcherState {
//
// When the dispatcher is in this state it will not enqueue
// more than `maxPreInitQueueSize` tasks.
Uninitialized,
Uninitialized = "Uninitialized",
// There are no commands queued and the dispatcher is idle.
Idle,
Idle = "Idle",
// The dispatcher is currently processing queued tasks.
Processing,
Processing = "Processing",
// The dispatcher is stopped, tasks queued will not be immediatelly processed.
Stopped,
Stopped = "Stopped",
// The dispatcher is shutdown, attempting to queue tasks while in this state is a no-op.
//
// This state is irreversible.
Shutdown,
Shutdown = "Shutdown",
}

// The possible commands to be processed by the dispatcher.
Expand Down Expand Up @@ -53,17 +53,30 @@ const enum Commands {
// A task the dispatcher knows how to execute.
type Task = () => Promise<void>;

// An executable command.
type Command = {
interface BaseCommand {
// The actual command to be executed.
command: Commands,
// A tag to use when logging the execution of the given command.
debugTag?: string,
}

interface TaskCommand extends BaseCommand {
task: Task,
command: Commands.Task | Commands.PersistentTask,
} | {
}

interface TestTaskCommand extends BaseCommand {
resolver: (value: void | PromiseLike<void>) => void,
task: Task,
command: Commands.TestTask,
} | {
}

interface InternalCommand extends BaseCommand {
command: Exclude<Commands, Commands.Task | Commands.TestTask | Commands.PersistentTask>,
};
}

// An executable command.
type Command = TaskCommand | TestTaskCommand | InternalCommand;

/**
* A task dispatcher for async tasks.
Expand All @@ -79,9 +92,7 @@ class Dispatcher {
// i.e. a Shutdown command is in the queue.
private shuttingDown = false;
// A promise containing the current execution promise.
//
// This is `undefined` in case there is no ongoing execution of tasks.
private currentJob?: Promise<void>;
private currentJob = Promise.resolve();

constructor(readonly maxPreInitQueueSize = 100, readonly logTag = LOG_TAG) {
this.queue = [];
Expand All @@ -100,11 +111,19 @@ class Dispatcher {
/**
* Executes a task safely, catching any errors.
*
* @param task The task to execute.
* @param task The task to execute.
* @param debugTag A tag identifiying the current task for debugging purposes.
*/
private async executeTask(task: Task): Promise<void> {
private async executeTask(task: Task, debugTag?: string): Promise<void> {
try {
await task();
log(
this.logTag,
[
"Done executing task in Task command:",
debugTag ? `[${debugTag}]` : "[unidentified]"
]
);
} catch(e) {
log(this.logTag, ["Error executing task:", JSON.stringify(e)], LoggingLevel.Error);
}
Expand All @@ -129,6 +148,13 @@ class Dispatcher {
private async execute(): Promise<void> {
let nextCommand = this.getNextCommand();
while(nextCommand) {
log(
this.logTag,
[
`Executing dispatched ${nextCommand.command} command:`,
nextCommand.debugTag ? `[${nextCommand.debugTag}]` : "[unidentified]"
]
);
switch(nextCommand.command) {
case(Commands.Stop):
this.state = DispatcherState.Stopped;
Expand All @@ -147,15 +173,19 @@ class Dispatcher {
nextCommand = this.getNextCommand();
continue;
case (Commands.TestTask):
await this.executeTask(nextCommand.task);
await this.executeTask(nextCommand.task, nextCommand.debugTag);
nextCommand.resolver();
nextCommand = this.getNextCommand();
continue;
case(Commands.PersistentTask):
case(Commands.Task):
await this.executeTask(nextCommand.task);
await this.executeTask(nextCommand.task, nextCommand.debugTag);
nextCommand = this.getNextCommand();
}

if (nextCommand) {
log(this.logTag, ["Getting the next command...", nextCommand.command]);
}
}
}

Expand All @@ -176,10 +206,15 @@ class Dispatcher {
// that was launched inside another task.
this.currentJob
.then(() => {
this.currentJob = undefined;
// eslint-disable-next-line @typescript-eslint/no-this-alias
const that = this;
if (this.state === DispatcherState.Processing) {
this.state = DispatcherState.Idle;
that.state = DispatcherState.Idle;
}
log(
this.logTag,
`Done executing tasks, the dispatcher is now in the ${this.state} state.`
);
})
.catch(error => {
log(
Expand Down Expand Up @@ -251,10 +286,12 @@ class Dispatcher {
* and the queues length exceeds `maxPreInitQueueSize`.
*
* @param task The task to enqueue.
* @param debugTag A tag identifiying the current task for debugging purposes.
*/
launch(task: Task): void {
launch(task: Task, debugTag?: string): void {
this.launchInternal({
task,
debugTag,
command: Commands.Task
});
}
Expand All @@ -264,10 +301,12 @@ class Dispatcher {
* but enqueues a persistent task which is not cleared by the Clear command.
*
* @param task The task to enqueue.
* @param debugTag A tag identifiying the current task for debugging purposes.
*/
launchPersistent(task: Task): void {
launchPersistent(task: Task, debugTag?: string): void {
this.launchInternal({
task,
debugTag,
command: Commands.PersistentTask
});
}
Expand All @@ -278,8 +317,9 @@ class Dispatcher {
* This is a no-op in case the dispatcher is not in an uninitialized state.
*
* @param task Optional task to execute before any of the tasks enqueued prior to init.
* @param debugTag A tag identifiying the current task for debugging purposes.
*/
flushInit(task?: Task): void {
flushInit(task?: Task, debugTag?: string): void {
if (this.state !== DispatcherState.Uninitialized) {
log(
this.logTag,
Expand All @@ -292,6 +332,7 @@ class Dispatcher {
if (task) {
this.launchInternal({
task,
debugTag,
command: Commands.Task
}, true);
}
Expand Down Expand Up @@ -374,7 +415,7 @@ class Dispatcher {
this.shuttingDown = true;
this.launchInternal({ command: Commands.Shutdown });
this.resume();
return this.currentJob || Promise.resolve();
return this.currentJob;
}

/**
Expand All @@ -388,7 +429,7 @@ class Dispatcher {
* @returns The promise.
*/
async testBlockOnQueue(): Promise<void> {
return this.currentJob && await this.currentJob;
return await this.currentJob;
}

/**
Expand Down
12 changes: 6 additions & 6 deletions glean/src/core/glean.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class Glean {
*/
private static async onUploadEnabled(): Promise<void> {
Context.uploadEnabled = true;
await Glean.coreMetrics.initialize(Glean.instance._config, Glean.platform, Context.metricsDatabase);
await Glean.coreMetrics.initialize(Glean.instance._config, Glean.platform);
}

/**
Expand Down Expand Up @@ -295,7 +295,7 @@ class Glean {
// so we need to know that state **before** scanning the pending pings
// to ensure we don't enqueue pings before their files are deleted.
await Context.pingsDatabase.scanPendingPings();
});
}, `${LOG_TAG}.initialize`);
}

static get serverEndpoint(): string | undefined {
Expand Down Expand Up @@ -358,7 +358,7 @@ class Glean {
await Glean.onUploadDisabled();
}
}
});
}, `${LOG_TAG}.setUploadEnabled`);
}

/**
Expand All @@ -374,7 +374,7 @@ class Glean {

// The dispatcher requires that dispatched functions return promises.
return Promise.resolve();
});
}, `${LOG_TAG}.setLogPings`);
}

/**
Expand All @@ -398,7 +398,7 @@ class Glean {

// The dispatcher requires that dispatched functions return promises.
return Promise.resolve();
});
}, `${LOG_TAG}.setDebugViewTag`);
}

/**
Expand All @@ -424,7 +424,7 @@ class Glean {

// The dispatcher requires that dispatched functions return promises.
return Promise.resolve();
});
}, `${LOG_TAG}.setSourceTags`);
}

/**
Expand Down
20 changes: 8 additions & 12 deletions glean/src/core/internal_metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import TimeUnit from "./metrics/time_unit.js";
import { generateUUIDv4 } from "./utils.js";
import type { ConfigurationInterface } from "./config.js";
import type Platform from "../platform/index.js";
import type MetricsDatabase from "./metrics/database.js";
import { Lifetime } from "./metrics/lifetime.js";
import log, { LoggingLevel } from "./log.js";
import { Context } from "./context.js";

const LOG_TAG = "core.InternalMetrics";

Expand Down Expand Up @@ -100,9 +100,9 @@ export class CoreMetrics {
});
}

async initialize(config: ConfigurationInterface, platform: Platform, metricsDatabase: MetricsDatabase): Promise<void> {
await this.initializeClientId(metricsDatabase);
await this.initializeFirstRunDate(metricsDatabase);
async initialize(config: ConfigurationInterface, platform: Platform): Promise<void> {
await this.initializeClientId();
await this.initializeFirstRunDate();
await StringMetricType._private_setUndispatched(this.os, await platform.info.os());
await StringMetricType._private_setUndispatched(this.osVersion, await platform.info.osVersion());
await StringMetricType._private_setUndispatched(this.architecture, await platform.info.arch());
Expand All @@ -114,12 +114,10 @@ export class CoreMetrics {
/**
* Generates and sets the client_id if it is not set,
* or if the current value is currepted.
*
* @param metricsDatabase The metrics database.
*/
private async initializeClientId(metricsDatabase: MetricsDatabase): Promise<void> {
private async initializeClientId(): Promise<void> {
let needNewClientId = false;
const clientIdData = await metricsDatabase.getMetric(CLIENT_INFO_STORAGE, this.clientId);
const clientIdData = await Context.metricsDatabase.getMetric(CLIENT_INFO_STORAGE, this.clientId);
if (clientIdData) {
try {
const currentClientId = createMetric("uuid", clientIdData);
Expand All @@ -141,11 +139,9 @@ export class CoreMetrics {

/**
* Generates and sets the first_run_date if it is not set.
*
* @param metricsDatabase The metrics database.
*/
private async initializeFirstRunDate(metricsDatabase: MetricsDatabase): Promise<void> {
const firstRunDate = await metricsDatabase.getMetric(
private async initializeFirstRunDate(): Promise<void> {
const firstRunDate = await Context.metricsDatabase.getMetric(
CLIENT_INFO_STORAGE,
this.firstRunDate
);
Expand Down
4 changes: 2 additions & 2 deletions glean/src/core/metrics/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ class MetricsDatabase {
}

const store = this._chooseStore(metric.lifetime);
const storageKey = await metric.identifier(this);
const storageKey = await metric.identifier();
for (const ping of metric.sendInPings) {
const finalTransformFn = (v?: JSONValue): JSONValue => transformFn(v).get();
await store.update([ping, metric.type, storageKey], finalTransformFn);
Expand Down Expand Up @@ -231,7 +231,7 @@ class MetricsDatabase {
metric: MetricType
): Promise<T | undefined> {
const store = this._chooseStore(metric.lifetime);
const storageKey = await metric.identifier(this);
const storageKey = await metric.identifier();
const value = await store.get([ping, metric.type, storageKey]);
if (!isUndefined(value) && !validateMetricInternalRepresentation<T>(metric.type, value)) {
log(
Expand Down
Loading