David Baron found this problem. David, please correct if I repeat incorrectly what Brian told me. The bug David found can lead to NSS being shut down too early, or too often. The reason is the PSM code that defines the nsNSSComponent module. This same component is listed for multiple contract IDs. Currently, nsNSSComponent is designed to be a singleton. If it is created multiple times, we run into big problems. While the code that explicitly asks for the nsNSSComponent uses GetService, thus accessing the singleton if it has been already created, some of the other contracts (e.g. the cert handlers) do not use this concept. Instead they use CreateInstace, which leads to the problems. Brian Ryner made the following suggestion: "My suggestion would be to split out nsNSSComponent into separate classes. Right now it's doing way too many things, and then some of those classes would not need to be singletons." For details, look at file nsNSSModule.cpp. Several classes use NS_NSSCOMPONENT_CID / nsNSSComponentConstructor.
I described the problem in a little more detail in attachment 55670 [details] [diff] [review] on bug 75947, in the comment in nsNSSModule.cpp near the end of the patch.
From the mentioned patch, a comment for the contract IDs in nsNSSModule.cpp. + // XXXldb Isn't the |nsNSSComponent| object supposed to be a singleton? + // These contract IDs are used with do_CreateInstance in nsURILoader.cpp, so + // their use will create additional instances of the nsNSSComponent object + // unless we modify nsNSSComponentConstructor to cache its singleton. (Will + // it then work correctly if it being used simultaneously by multiple users + // for this purpose?)
In my opinion it is a bug that NS_NSSCOMPONENT_CID is registered to implement the different contracts starting with NS_CONTENT_HANDLER_CONTRACTID_PREFIX, used by nsURILoader. nsURIILoader tries to create an instance for nsIContentHandler. However, neither nsNSSComponent (which is currently registered for doing that), nor any other class in security does implement this interface. I suggest that we remove these four registrations from the code as they have no effect. I believe that nsURILoader will try to instantiate that class, use QueryInterface, but fail because of a lack of support for the requested interface. The real mime type handling for downloading certs is handled by PSMContentListener. I will attach a patch to remove that code.
Created attachment 57164 [details] [diff] [review] Suggested fix Together with the patch in bug 75947 this will eliminate all additional registrations for class nsNSSComponent, fixing this bug.
Javi, while we talked about this patch, you suggested to try out whether obtaining certificates from Verisign still works. I tested it, it works. Can you please review?
Should 10977 block this?
Javi, I assume you meant bug 109777 ? I don't think this blocking is necessary. Bug 109777 blocks bug 75947. I think this is sufficient, as the code removed by this patch does currently not have any positive effect (as the registered class does not implement nsIContentHandler), only the negative effect of causing additional instances of nsNSSComponent being created.
Status: NEW → ASSIGNED
Comment on attachment 57164 [details] [diff] [review] Suggested fix r=javi (since there is a new bug to ensure cert downloading works.)
Comment on attachment 57164 [details] [diff] [review] Suggested fix sr=blizzard
Attachment #57164 - Flags: superreview+
checked in => fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Verified per kaie's comment.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.