Random X errors caused by recent cursor patch?

[Dan Kegel]

Looks like there's been a regression lately.  I'm
getting errors like this

X Error of failed request:  BadPixmap (invalid Pixmap parameter)
  Major opcode of failed request:  54 (X_FreePixmap)
  Resource id in failed request:  0x2a0006a

randomly, about every fourth time I run things that used to
work.  A user on c.e.m.w.
http://groups.google.com/group/comp.emulators.ms-windows.wine/msg/1f41dbd21a...
speculates that the recent cursors patch
introduced the regression.

The error doesn't happen often enough to make
a regression test easy, and I haven't tried myself yet.

[Huw Davies]

On Tue, Apr 03, 2007 at 08:26:59AM -0700, Dan Kegel wrote:
> Looks like there's been a regression lately.  I'm
> getting errors like this
>
> X Error of failed request:  BadPixmap (invalid Pixmap parameter)
>  Major opcode of failed request:  54 (X_FreePixmap)
>  Resource id in failed request:  0x2a0006a
>
> randomly, about every fourth time I run things that used to
> work.  A user on c.e.m.w.
> http://groups.google.com/group/comp.emulators.ms-windows.wine/msg/1f41dbd21a...
> speculates that the recent cursors patch
> introduced the regression.
>
> The error doesn't happen often enough to make
> a regression test easy, and I haven't tried myself yet.

Hi Dan,

Could you see if this helps?  It looks like XRenderFreePicture
actually destroys the underlying pixmap, so we ended up freeing it
twice.

Thanks,
Huw.

[John Smith]

I was able to reproduce the badpixmap bug on peachtree 2006 right after
picking a company from the "open company" dialog.  This patch seemed to fix
this (or atleast hide it from being as reproducible as it was).  I had some
trouble applying it to current gitwine, but I typed it in manually and it
worked fine.

john

On 4/3/07, Huw Davies  wrote:
> On Tue, Apr 03, 2007 at 08:26:59AM -0700, Dan Kegel wrote:
> > Looks like there's been a regression lately.  I'm
> > getting errors like this
> >
> > X Error of failed request:  BadPixmap (invalid Pixmap parameter)
> >  Major opcode of failed request:  54 (X_FreePixmap)
> >  Resource id in failed request:  0x2a0006a
> >
> > randomly, about every fourth time I run things that used to
> > work.  A user on c.e.m.w.
>
> http://groups.google.com/group/comp.emulators.ms-windows.wine/msg/1f41dbd21a...
> > speculates that the recent cursors patch
> > introduced the regression.
> >
> > The error doesn't happen often enough to make
> > a regression test easy, and I haven't tried myself yet.
>
> Hi Dan,
>
> Could you see if this helps?  It looks like XRenderFreePicture
> actually destroys the underlying pixmap, so we ended up freeing it
> twice.
>
> Thanks,
> Huw.
> --
> Huw Davies
> [e-mail]
> > From 8a76cf26395043428c24851d675c219735bcf502 Mon Sep 17 00:00:00 2001
>
> From: Huw Davies 
> Date: Tue, 3 Apr 2007 17:12:32 +0100
> Subject: winex11.drv: XRenderFreePicture destroys the underlying storage,
> so don't free the pixmap.
> To: wine-patches
>
> dlls/winex11.drv/xrender.c |   22 +++++++++-------------
> 1 files changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/dlls/winex11.drv/xrender.c b/dlls/winex11.drv/xrender.c
> index e03540e..6b86c74 100644
> --- a/dlls/winex11.drv/xrender.c
> +++ b/dlls/winex11.drv/xrender.c
> @@ -85,7 +85,6 @@ struct tagXRENDERINFO
>      int                cache_index;
>      Picture            pict;
>      Picture            tile_pict;
> - Pixmap             tile_xpm;
> COLORREF           lastTextColor;
>
> };
>
> @@ -596,7 +595,6 @@ void X11DRV_XRender_UpdateDrawable(X11DRV_PDEVICE
> *physDev)
>      if(physDev->xrender->pict)
>      {
>          TRACE("freeing pict = %lx dc = %p\n", physDev->xrender->pict,
> physDev->hdc);
> -
>    XFlush(gdi_display);
>    pXRenderFreePicture(gdi_display, physDev->xrender->pict);
>    physDev->xrender->pict = 0;
> }
>
> @@ -605,11 +603,6 @@ void X11DRV_XRender_UpdateDrawable(X11DRV_PDEVICE
> *physDev)
>          pXRenderFreePicture(gdi_display, physDev->xrender->tile_pict);
>          physDev->xrender->tile_pict = 0;
>      }
> -
>
> if(physDev->xrender->tile_xpm)
> -
>
> {
> -
>    XFreePixmap(gdi_display, physDev->xrender->tile_xpm);
> -
>    physDev->xrender->tile_xpm = 0;
> -
>
> }
>
> wine_tsx11_unlock();
>
> @@ -1186,18 +1179,21 @@ BOOL X11DRV_XRender_ExtTextOut( X11DRV_PDEVICE
> *physDev, INT x, INT y, UINT flag
>  if(X11DRV_XRender_Installed) {
>      /* Create a 1x1 pixmap to tile over the font mask */
> -
>    if(!physDev->xrender->tile_xpm) {
> -
>    if(!physDev->xrender->tile_pict) {
>       XRenderPictureAttributes pa;
> -
>        Pixmap xpm;
>
>       XRenderPictFormat *format = (physDev->depth == 1) ?
>
> mono_format : screen_format;
>             wine_tsx11_lock();
> -
>       physDev->xrender->tile_xpm = XCreatePixmap(gdi_display,
> -
>                                                  physDev->drawable,
> -
>                                                  1, 1,
> -
>                                                  format->depth);
> -
> -
>        /* The pixmap will be freed with XRenderFreePicture */
> -
>        xpm = XCreatePixmap(gdi_display,
> -
>                            physDev->drawable,
> -
>                            1, 1,
> -
>                            format->depth);
>       pa.repeat = True;
>       physDev->xrender->tile_pict =
>
> pXRenderCreatePicture(gdi_display,
> -
>
> physDev->xrender->tile_xpm,
> -
>                                                           xpm,
>                                                           format,
>                                                           CPRepeat,
>
> &pa);
>             wine_tsx11_unlock();
> --
> 1.5.0.5

[Tom Spear]

On 4/5/07, John Smith  wrote:
> This patch seemed to fix this (or atleast hide it from being as reproducible as it
> was).

Hide it from being as reproducible?  Are you still able to reproduce
it at all, now?

[Dan Kegel]

On 4/3/07, Huw Davies  wrote:
> > X Error of failed request:  BadPixmap (invalid Pixmap parameter)
>
> Could you see if this helps?  It looks like XRenderFreePicture
> actually destroys the underlying pixmap, so we ended up freeing it
> twice.

I finally ran into the bug again, so I applied your patch
to this morning's wine.  The problem didn't recur.
This doesn't prove much, since the bug is so fickle,
but at least your patch didn't break anything.

[Klaus Layer]

Am Dienstag, 3. April 2007 18:15 schrieb Huw Davies:
> Hi Dan,
>
> Could you see if this helps?  It looks like XRenderFreePicture
> actually destroys the underlying pixmap, so we ended up freeing it
> twice.
>
> Thanks,
> Huw.
> -- 
> Huw Davies
> [e-mail]

Hi Huw,

other than Dan for me the error happened any time. After applying your patch 
it works.

thanks,

Klaus

[Dan Kegel]

On 4/3/07, Huw Davies  wrote:
> > X Error of failed request:  BadPixmap (invalid Pixmap parameter)
> >  Major opcode of failed request:  54 (X_FreePixmap)
> >  Resource id in failed request:  0x2a0006a
>
> Could you see if this helps?  It looks like XRenderFreePicture
> actually destroys the underlying pixmap, so we ended up freeing it
> twice.

Hi Huw,
when are you going to submit that fix?  People keep tripping over it;
see http://bugs.winehq.org/show_bug.cgi?id=7959 and
http://bugs.winehq.org/show_bug.cgi?id=8041
...

[Huw Davies]

On Sat, Apr 14, 2007 at 05:19:31AM -0700, Dan Kegel wrote:
> On 4/3/07, Huw Davies  wrote:
> > > X Error of failed request:  BadPixmap (invalid Pixmap parameter)
> > >  Major opcode of failed request:  54 (X_FreePixmap)
> > >  Resource id in failed request:  0x2a0006a
> >
> > Could you see if this helps?  It looks like XRenderFreePicture
> > actually destroys the underlying pixmap, so we ended up freeing it
> > twice.
>
> Hi Huw,
> when are you going to submit that fix?  People keep tripping over it;
> see http://bugs.winehq.org/show_bug.cgi?id=7959 and
> http://bugs.winehq.org/show_bug.cgi?id=8041

Unfortunately that patch isn't the correct fix - looking at the
XServer code it's clear that the Pixmap is correctly ref counted, so
we do indeed need to call XFreePixmap.

I think the XError is due to a race between the thread display and the
gdi display connections, but I haven't yet understood the issue well
enough to write a correct patch.

Huw.

[Lei Zhang]

On 4/16/07, Huw Davies  wrote:
> On Sat, Apr 14, 2007 at 05:19:31AM -0700, Dan Kegel wrote:
> > On 4/3/07, Huw Davies  wrote:
> > > > X Error of failed request:  BadPixmap (invalid Pixmap parameter)
> > > >  Major opcode of failed request:  54 (X_FreePixmap)
> > > >  Resource id in failed request:  0x2a0006a
> > >
> > > Could you see if this helps?  It looks like XRenderFreePicture
> > > actually destroys the underlying pixmap, so we ended up freeing it
> > > twice.
> >
> > Hi Huw,
> > when are you going to submit that fix?  People keep tripping over it;
> > see http://bugs.winehq.org/show_bug.cgi?id=7959 and
> > http://bugs.winehq.org/show_bug.cgi?id=8041
>
> Unfortunately that patch isn't the correct fix - looking at the
> XServer code it's clear that the Pixmap is correctly ref counted, so
> we do indeed need to call XFreePixmap.
>
> I think the XError is due to a race between the thread display and the
> gdi display connections, but I haven't yet understood the issue well
> enough to write a correct patch.
> Huw.
>
> Huw Davies
> [e-mail]

BTW, I was able to frequently reproduce the bug while running the
user32 message test.