Re: [Gimp-print-devel] [Patch] gutenprint53+usb crashes with libusb-1.0.25

[Benjamin B. <bb...@re...>]

Hi,

On Wed, 2022-02-23 at 09:34 +0100, Zdenek Dohnal wrote:
> thank you for looking into the issue!
> 
> On 2/22/22 19:00, Solomon Peachy wrote:
> > On Tue, Feb 22, 2022 at 07:55:18AM +0100, Zdenek Dohnal wrote:
> > > there were changes in libusb 1.0.25 which is in Fedora 36 - f.e. default USB
> > > context is not initialized unless you specifically pass NULL into
> > > `libusb_init()` - gutenprint53+usb backend actually depended on this hidden
> > > behavior, because it passes its own context address into `libusb_init()`
> > > instead of NULL. So the change causes the backend to crash if you run it:
> > Yikes!
> > 
> > > According libusb maintainer, Ben Berg, the backend can work with the default
> > > USB context and he prepared the patch for it (the file is attached). I was
> > > able to verify the patch fixes the crash, but I don't have a device
> > > supported by gutenprint itself, so I couldn't do an additional sanity
> > > testing. The patch looks good to me though.
> > Will this work for multi-threaded stuff where there could conceivably be
> > multiple contexts in use?  That's not being exercised in the current
> > gutenprint code but some of the stuff I have cooking will change that.
> 
> Ben, would you mind looking into it? IMO we probably cannot use a 
> default USB context for multi-threading...
> 
> In such case, IMO we can init the default context just for the 
> print_help() scope and close it after we're done with it.
> 
> You can join us in discussion on the bug if you are able as well [1], 
> I've pinged Robert there as well, because I didn't know your email for 
> bugzilla...

I am already subscribed to that bug. :)

About multi-threading. libusb itself is thread-safe, so, it depends on
what exactly the needs are. Having separate contexts means that you can
choose to only process events on one of them at a time. This avoids any
issue where one thread is suddenly processing events for another (i.e.
it is undefined which thread executes the callbacks for asynchronous
transfers).

But, e.g. just doing an enumeration from a different thread using the
same context should be completely fine.

Benjamin

> > 
> > > Would you mind adding the patch to the project if it looks good for you?
> > I won't be able to physically test any of this until this coming weekend
> > (out of town and while I have remote access everything pertinent is
> > turned off) but when I'm back, I'll be able to at least it doesn't
> > introduce any regressions on F34 & F35.
> Ok, Ben reverted the libusb change for now to do not block Fedora 36 
> beta, so IMO after weekend is fine :) .
> > 
> >   - Solomon
> 
> 
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=2055504
>