<AWT Dev> status of 6532373 (xcb_xlib.c:50: xcb_xlib_unlock: Assertion 'c->xlib.lock' failed.)

[Josh Triplett]

Oleg Sukhodolsky wrote:
> JFYI, I have developed preliminary version of the fix (some lines may be 
> changed in final version because of possible conflicts with removing 
> Motif-based AWT implementation)
> 
> But anyone who suffer from the problem can already use these changes.

Thank you for looking at this.  Comments on the patch below; note that
other than the comment about using the major numer in the library
name, none of them absolutely need fixing before applying the bugfix.

> +++ awt_GraphicsEnv.c        2007-09-25 13:06:28.000000000 +0400
> @@ -30,10 +30,11 @@
>   #include <sun_awt_X11GraphicsEnvironment.h>
>   #include <sun_awt_X11GraphicsDevice.h>
>   #include <sun_awt_X11GraphicsConfig.h>
>   #ifndef HEADLESS
>   #include <X11/extensions/Xdbe.h>
> +#include <X11/XKBlib.h>

XKBlib?  This change doesn't seem related to the rest of the patch.

And also, wouldn't that make the later inclusion of the same header
under __linux__ redundant?  (Unless this block resides under an #ifdef
outside the context.)

> +static void xinerama_init_linux()
> +{
> +    void* libHandle = 0;
> +    char* XineramaLibName= "libXinerama.so";

Given that you expect a particular interface from this library, you
should include the major number in this name: "libXinerama.so.1".
Otherwise, if the system you run on has an incompatible version of
this library (such as a new libXinerama.so.2, or an old
libXinerama.so.0), Java would break mysteriously at runtime by trying
to use the old interface in the new library.  If you include the major
number, you will find the library with the version you expect, or you
will not find the library at all.  Furthermore, distribution packages
often split libraries into a runtime package and a development package
(for instance, in Debian, libxinerama1 and libxinerama-dev), and only
the development package includes the symlink for the unversioned
library name; thus, if you use "libXinerama.so", distributions either
need to patch Java to use the versioned library name or make Java
depend on the development packages for libXinerama.

> +    /* load library */
> +    libHandle = dlopen(XineramaLibName, RTLD_LAZY | RTLD_GLOBAL);
> +    if (libHandle != 0) {
> +        XineramaQueryScreens = (XineramaQueryScreensFunc*)
> +            dlsym(libHandle, XineramaQueryScreensName);
> +
> +        if (XineramaQueryScreens != NULL) {
> +            DTRACE_PRINTLN("calling XineramaGetInfo func on Linux");

Wrong function name in the string here.  Should say XineramaQueryScreens.

> -#else /* Solaris */
> -
> -    char* XinExtName = "XINERAMA";
> -    int32_t major_opcode, first_event, first_error;
> -    Bool gotXinExt = False;
> +}
> +#endif
> +#ifndef __linux__ /* Solaris */
> +static void xinerama_init_solaris()
> +{
>       void* libHandle = 0;
> -    unsigned char fbhints[MAXFRAMEBUFFERS];
> -    int locNumScr = 0;
> -
>       char* XineramaLibName= "libXext.so";

Same comment as before about using the major number, but this bug
already exists so fixing it doesn't form part of this bugfix patch.

> +/*
> + * Checks is Xinerama is running and perform Xinerama-related

Typo: s/Checks is/Checks if/.

> + * platform dependent initialization.
> + */
> +static void xineramaInit(void) {
> +    char* XinExtName = "XINERAMA";
> +    int32_t major_opcode, first_event, first_error;
> +    Bool gotXinExt = False;
> +
> +    gotXinExt = XQueryExtension(awt_display, XinExtName, &major_opcode,
> +                                &first_event, &first_error);
> +
> +    if (!gotXinExt) {
> +        DTRACE_PRINTLN("Xinerama is not available");
> +        return;
> +    }
> +
> +    DTRACE_PRINTLN("Xinerama extension is available");

The two strings here seem inconsistent: one says "Xinerama" and the
other says "Xinerama extension".

>   Java_sun_awt_X11GraphicsDevice_getDisplay(JNIEnv *env, jobject this)
>   {
>   #ifdef HEADLESS
>       return NULL;
>   #else
> -    return (jlong) awt_display;
> +    return ptr_to_jlong(awt_display);

This change doesn't seem related to the rest of the patch.

- Josh Triplett

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 252 bytes
Desc: OpenPGP digital signature
Url : http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20070928/c8be8546/signature.asc