101005 - [meta] Review *all* PSM code for threadsafety issues

Status: NEW

(Core :: Security: PSM, defect, P3)

Description

[(Don't use, old account) - Kai Engert]

PSM uses the NS_IMPL_THREADSAFE_ISUPPORTS... marcos.

I believe, using these macros says: "it is safe to call any methods from any
thread". (No debug output / warning messages will be shown if this happens, as
it is considered safe.)
I learned in chats, that these macros might have been used to make the
assertions go away, but the code itself might have not been fully checked for
multithread safety. Currently, PSM doesn't use mutexes / locks.

We should review all PSM code to make sure we are not introducing problems that
lead to memory corruption etc, or are the cause for bugs we see but can't explain.

It might be helpful to replace the NS_IMPL_THREADSAFE macros with the
non-threadsafe versions, and look at the assertions that are shown (I made a
small test doing so several days ago and saw a bunch of assertions).

Once we know how much protection is needed, we can decide whether we prefer
locks or adding thread proxies to code that calls PSM from different threads.

Comment 1

[(Don't use, old account) - Kai Engert]

David suggested this should be -> P1.

Comment 2

[Bradley Baetz (:bbaetz)]

Currently, nsNSSComponent is used by both the ui thread and the socket transport
thread, as I understand it.

Comment 3

[(Don't use, old account) - Kai Engert]

Changing my prefered e-mail address.

Comment 4

[John G. Myers]

The CRL update code appears to have thread safety issues--see bug 200119.

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

This should be more of a meta-bug that tracks smaller, more achievable sub-tasks that will eventually get us to a more consistent state with respect to thread safety in PSM.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The meta keyword is there, the bug doesn't depend on other bugs and there is no activity for 12 months.
:keeler, maybe it's time to close this bug?

Comment 7

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

blocks -> depends on