279361 - nsAutoLock's use of DEBUG may prevent (dynamic) linking of components

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect)

Description

[David Teller [:Yoric] - still alive but not very active]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.7.5) Gecko/20050110 Firefox/1.0 (Debian package 1.0+dfsg.1-2)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.7.5) Gecko/20050110 Firefox/1.0 (Debian package 1.0+dfsg.1-2)

The file xpcom/nsAutoLock.h is very dependent on the DEBUG flag.

Many extension developers have the DEBUG flag set when writing their code.
Unfortunately, when that flag is set, using nsAutoLock.h produces a class which
requires the following symbols to be implemented :

nsAutoLockBase::nsAutoLockBase(void* addr, nsAutoLockType type);
nsAutoLockBase::~nsAutoLockBase();

As these symbols are only implemented in Debug builds of Mozilla, components
using nsAutoLock cannot link and fail almost silently at runtime.

Undefining DEBUG during the inclusion of nsAutoLock.h is not a solution as the
following methods stop cannot compile in this particular setup :

    ~nsAutoMonitor() {
        NS_ASSERTION(mMonitor, "null monitor");
        if (mMonitor && mLockCount) {
#ifdef DEBUG
            PRStatus status =
#endif
            PR_ExitMonitor(mMonitor);
            NS_ASSERTION(status == PR_SUCCESS, "PR_ExitMonitor failed");
        }
    }


~nsAutoCMonitor() {
        if (mLockCount) {
#ifdef DEBUG
            PRStatus status =
#endif
            PR_CExitMonitor(mLockObject);
            NS_ASSERTION(status == PR_SUCCESS, "PR_CExitMonitor failed");
        }
    }




In order to solve the problem, a quick hack might be to rewrite the
aforementioned methods as follows:

    ~nsAutoMonitor() {
        NS_ASSERTION(mMonitor, "null monitor");
        if (mMonitor && mLockCount) {
#ifdef DEBUG
            PRStatus status =
#endif
            PR_ExitMonitor(mMonitor);
#ifdef DEBUG
            NS_ASSERTION(status == PR_SUCCESS, "PR_ExitMonitor failed");
#endif
        }
    }


~nsAutoCMonitor() {
        if (mLockCount) {
#ifdef DEBUG
            PRStatus status =
#endif
            PR_CExitMonitor(mLockObject);
#ifdef DEBUG
            NS_ASSERTION(status == PR_SUCCESS, "PR_CExitMonitor failed");
#endif
        }
    }


Reproducible: Always

Steps to Reproduce:
(unsure as what to fill here, as the errors are either compilation errors or
silent dynamic linking errors)

Comment 1

[Darin Fisher]

nsAutoLock is not meant to be used by extension authors.

Comment 2

[David Teller [:Yoric] - still alive but not very active]

(In reply to comment #1)
> nsAutoLock is not meant to be used by extension authors.

What should we use then ?
I mean, we need some thread safety, too.

Comment 3

[Darin Fisher]

NSPR provides PR_Lock, PR_Unlock, etc.  Those are exposed for extension authors
and embedders to use.  nsAutoLock is one of those private utility classes used
by the Mozilla implementation.  As you may have experienced, many of these
private APIs fluctuate from release to release, so if you wish to build a binary
extension that continues to work with future versions of Mozilla-based products
(i.e., Firefox), then you should take care to only rely on "frozen" APIs.  A
good solution is to get the Gecko SDK, and build your extension using that.  If
you find yourself needing interfaces that are not included in the Gecko SDK,
then you will know that you are potentially relying on a private interface that
may change in the future.  Now, there are quite a few interfaces that are stable
and will hopefully be added to the Gecko SDK in the near future, so whenever you
encounter a need for one of the interfaces not found in the Gecko SDK, I suggest
that you ask on irc.mozilla.org#developers or in n.p.m.embedding to see if it is
one of those interfaces that is nearly frozen.  If it isn't, then it is of
course instructive for us to know that you need the interface because then we
can work to provide it.

Some more info on the Gecko SDK here:
http://wiki.mozilla.org/wiki/Gecko:SDK (very incomplete)

Comment 4

[David Teller [:Yoric] - still alive but not very active]

I am perfectly aware that, by using mozilla's source (actually mozilla-dev)
rather than gecko-sdk, I might be the sudden victim of interface changes. And I
know how to rewrite nsAutoLock if I need to, it's a quite standard design, after
all. I'm just suggesting a quick hack to remove an oddity for some application
developers.

Comment 5

[David Teller [:Yoric] - still alive but not very active]

Anyway, I tend to believe that such a small suggestion does not deserve the
energy we're putting in the discussion. So, do as you see fit and keep up the
good work on the important parts of Mozilla :)

Comment 6

[Gervase Markham [:gerv]]

This is an automated message, with ID "auto-resolve01".

This bug has had no comments for a long time. Statistically, we have found that
bug reports that have not been confirmed by a second user after three months are
highly unlikely to be the source of a fix to the code.

While your input is very important to us, our resources are limited and so we
are asking for your help in focussing our efforts. If you can still reproduce
this problem in the latest version of the product (see below for how to obtain a
copy) or, for feature requests, if it's not present in the latest version and
you still believe we should implement it, please visit the URL of this bug
(given at the top of this mail) and add a comment to that effect, giving more
reproduction information if you have it.

If it is not a problem any longer, you need take no action. If this bug is not
changed in any way in the next two weeks, it will be automatically resolved.
Thank you for your help in this matter.

The latest beta releases can be obtained from:
Firefox:     http://www.mozilla.org/projects/firefox/
Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html
Seamonkey:   http://www.mozilla.org/projects/seamonkey/

Comment 7

[Serge Gautherie (:sgautherie)]

R.WontFix (== "Invalid"), per comment 5.