Closed Bug 1413009 Opened 3 years ago Closed 2 years ago

Configure parent main thread message filter to only accept one logical thread at a time

Categories

(Core :: IPC: MSCOM, enhancement)

Unspecified
Windows
enhancement
Not set
normal

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox58 --- affected
firefox59 --- ?

People

(Reporter: aklotz, Assigned: aklotz)

Details

(Whiteboard: aes+)

Attachments

(1 file)

We can modify our message filter to force remote clients to defer their requests if we are already processing an existing client. This should prevent some reentry scenarios that are problematic for us in a11y.
Comment on attachment 8923578 [details] [diff] [review]
Prevent multiple remote clients from interleaving

I'm concerned this is not going to behave the way it might seem. I think RetryRejectedCall gets called on the *client's* IMessageFilter, not the server's:

"COM calls RetryRejectedCall on the caller's IMessageFilter interface immediately after receiving SERVERCALL_RETRYLATER or SERVERCALL_REJECTED from the IMessageFilter::HandleInComingCall method on the callee's IMessageFilter interface."
(https://msdn.microsoft.com/en-us/library/windows/desktop/ms680739(v=vs.85).aspx)

(As a side note, that article also notes you can return a value 0 ≤ value < 100 to retry immediately, which is probably better than 100 ms.)

So, your implementation of RetryRejectedCall is never going to get called. Instead, COM will try to call the client's implementation of this method (if any). Unfortunately, most clients probably don't implement it.

The documentation isn't clear what the default message filter does. It says:

"Applications should silently retry calls that have returned with SERVERCALL_RETRYLATER."

However, it's unclear whether this is just the recommendation as to how clients should implement or whether this is also how the default IMessageFilter implementation works.

I'd be uncomfortable taking this unless we can prove that it behaves correctly on the client side. Unfortunately, that's going to be fairly difficult in this short time frame... unless you've already done this/have ideas on how to do it?
Ah, yes, I see HandleInComingCall is the only one we should be handling.

But I also agree that this then makes things interesting for clients.
Attachment #8923578 - Flags: review?(jteh)
I don't know of a solution to comment 1, nor do we have any evidence that this solves a current problem. Closing.
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
(In reply to James Teh [:Jamie] from comment #5)
> I don't know of a solution to comment 1,

I meant comment 2; i.e. that clients probably don't implement RetryRejectedCall.
You need to log in before you can comment on or make changes to this bug.