952692 - Nuwa process can accumulate IPC messages

Status: RESOLVED WORKSFORME

(Core :: IPC, defect, P1)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I believe it is possible for the Nuwa process to accumulate IPC messages that are broadcast to all processes.  This means that every time the Nuwa process forks and "wakes up" all of those messages will be processed.  This could slow down app startup (especially if memory-pressure notifications which trigger GC/CC are queued up).  We should probably not broadcast messages to the Nuwa process.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Patrick, is this something you can take?  Just trying to make sure we have coverage on 1.3+ bugs.  Thanks!

Comment 2

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Nuwa can process IPC message even when it is ready for forking new process. We are trying to figure out if this really happen.

Comment 3

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: test-ipc

I added some log to look the length of mPending in MessageChannel. After Nuwa process is frozen, the MessageQueue can still go back to zero.

I think that all messages dispatched to main thread can be processed. Only when the message makes main thread try to acquire lock which a frozen thread holds, it would cause Nuwa frozen. (Like https://bugzilla.mozilla.org/show_bug.cgi?id=941466#c0)

Comment 4

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: message queue log

Comment 5

[[:fabrice] Fabrice Desré]

Patrick, is there any follow up here?

Comment 6

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Based on the log, I don't think IPC messages are queued when Nuwa is frozen. Kyle, do you think we need to do more investigation?

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Yes, you are correct.  It is however possible for those IPC messages to result in queueing up events on the main thread such as memory pressure notifications which will have to run every time we fork the Nuwa process.

I will take this.

Comment 8

[Cervantes Yu [:cyu] [:cervantes]]

For memory pressure, the Nuwa process just ignores it by returning true in ContentChild::RecvFlushMemory(). So the remaining problem is: is it possible that the main thread's queue has pending IPC message to run ContentChild::RecvFlushMemory() where it is supposedly to be ignored but the delayed execution of IsNuwaProcess() becomes false in the forked child?

This is how the NuwaFork() request runs in the Nuwa process:
1. The IPC thread receives the request and dispatch ContentChild::RecvNuwaFork() to the main thread.
2. The main thread dispatches RunNuwaFork() to the IPC thread.
3. RunNuwaFork() dispatch a runnable to the main thread to fork(). This is synchronous. The IPC thread is blocked until the main thread finishes fork().

So the main thread's run queue should be empty when it forks, even we receive GC requests between 2 and 3. It is possible that other threads might have runnables queued up when it is frozen. But they are unlikely, if ever possible, the GC requests.

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Ah, yes, you are correct.  Guess I should read code before filing bugs ;)