[PATCH] libusb pthread usage causes error in garbage collected Cocoa apps

[rogueresearch]

When libusb is used in a Mac OS X Cocoa application that uses garbage collection, an error is logged by the OS:

malloc: * auto malloc[22178]: error: GC operation on unregistered thread. Thread registered implicitly. Break on auto_zone_thread_registration_error() to debug.

Although libusb does not use Objective-C, it does use CoreFoundation? which in turn uses Objective-C. CoreFoundation? objects participate in Obj-C garbage collection, and so in a way libusb does too.

Anyway, the fix is easy: Whenever libusb creates a pthread, that thread must register itself with the garbage collector by calling objc_registerThreadWithCollector().

I am testing a patch currently...

Note that this will require linking to an additional lib, namely libobjc.dylib. I wouldn't worry about that though, because libusb already links to CoreFoundation?.framework, which as you can see below, already links to libobjc:

$ otool -L /System/Library/Frameworks/CoreFoundation?.framework/CoreFoundation:

/System/Library/Frameworks/CoreFoundation?.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 550.42.0)
/usr/lib/libauto.dylib (compatibility version 1.0.0, current version 1.0.0)
/usr/lib/libicucore.A.dylib (compatibility version 1.0.0, current version 40.0.0)
/usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 227.0.0)
/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.3)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 125.2.1)

[rogueresearch]

I've attached a patch for others to review. I don't know anything about autotools, so I'm not too sure where to add the -lobjc linker flag.

[hjelmn]

libusb may be linked against the CoreFoundation? framework but it does not use any core foundation objects so not registering the work thread should not cause any problems. Not registering will, however, cause the annoying warning message you are seeing to be printed. That is enough of a reason to accept this patch.

*edit* I should clarify my comment. The thread does use some core foundation objects BUT I don't think those objects should cause any issues with the objc garbage collector. The cf objects are owned by the thread and should not be autoreleased.

[rogueresearch]

Nathan, thanks for looking at this and agreeing to integrate this patch.

A nitpick: it is not an "annoying warning message", it is an _error_ message. The API contract requires registering the thread with the collector, and I'm told this may become a hard error in the future. Cheers, Sean.

[rogueresearch]

Nathan, did you commit this?

A small change is needed. We should not, in fact, call the unregister function at the end of the thread. Apple has clarified this here: ​http://osdir.com/ml/cocoa-dev/2010-10/msg00759.html

[stuge]

This is not in libusb.git yet, and I also don't think Nathan has approved the patch, but I'm not sure.

In any case, could you update the patch? Please upload a new file with the same name, and check the "Replace existing attachment of the same name" box.

Thanks!

[rogueresearch]

patch for bug #63

[rogueresearch]

stuge, I have updated the patch.

I'm confident in the code change, but not so much in my change to configure.ac, since I know nothing about autotools.

[segher]

On a 10.3 system:

$ otool -L /System/Library/Frameworks/CoreFoundation?.framework/Versions/A/CoreFoundation
/System/Library/Frameworks/CoreFoundation?.framework/Versions/A/CoreFoundation:

/System/Library/Frameworks/CoreFoundation?.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 299.39.0)
/usr/lib/libicucore.A.dylib (compatibility version 1.0.0, current version 26.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 71.1.5)

so please do not link libobjc unconditionally.

[rogueresearch]

segher, as I said, I know nothing of autotools, so don't know how one would link conditionally. Also, even linking libojbc unconditionally would not be so bad, as the code change is conditional to 10.6+.

What is libusb's minimum supported version of Mac OS X? I'm surprised you care about 10.3.

[segher]

segher, as I said, I know nothing of autotools,

I don't know much about it either.

so don't know how one would link conditionally.

Also, even linking libojbc unconditionally would not be so bad, as the code change is conditional to 10.6+.

It links a huge set of libraries for no reason at all. This would be no
problem with static libs, since those are just ignored at link time if
unused; but this isn't true with dynamic libs.

This is costly in startup time for all programs linked against libusb.
It has some size cost as well, not worried about that though.

What is libusb's minimum supported version of Mac OS X? I'm surprised you care about 10.3.

10.3 works fine (after some patches Peter Stuge did). I use libusb on it
almost daily. I'm not so sure 10.2 will work, it has a few idiosyncracies
(but fewer than 10.5 and 10.6!)

I'll have a look at the autoXXXX. No promises :-)

[rogueresearch]

segher, if someone can find a way to link to libojbc only on 10.6 then I'm all for it.

But, as I said, libusb already links to CoreFoundation? which itself links to libojbc. This is true on 10.6, 10.5, and 10.4 (I checked with otool). True that it's not the case on 10.3 (and presumably earlier).

Do you not agree that it's more important to fix a bug related to the current OS even if in exchange for a tiny speed penalty on a 7 year old OS? It's not as if libobjc isn't present on 10.3.

[segher]

Ah, already on 10.4. I withdraw my objections.

I find this whole solution terribly ugly though. Not that I see
any better way to fix the problem :-(

Is the attached patch still current? I'll test it on 10.3 and 10.4.

Thanks!

[rogueresearch]

Yes, the patch is current.

The ugliness is unavoidable unless we want to require the 10.6 SDK. That method's declaration was only added in the 10.6 SDK.

Thanks for trying it on 10.3 and 10.4!

[segher]

With "ugliness" i mean the whole concept of having to call
objc_registerThreadWithCollector(). Yuck! The #ifdef
stuff isn't so bad, in comparison. But, as I said, it seems
unavoidable.

[segher]

The patch doesn't cause any new problems on 10.3 as far as I can
see. It does however add some trailing whitespace in configure.ac;
fix that?

[rogueresearch]

I don't see any trailing whitespace added...

[segher]

The two "empty" lines around the "Tell the Objective-C ..."
block contain two spaces each.

[rogueresearch]

ah, that's not configure.ac :) So the libusb convention is for 'blank' lines not to be indented to the current level I guess? Can I simply remove the spaces in the patch file, or must I actually use git to generate a new patch?

[segher]

Oh, different file? Sorry!

The convention is to not have any blanks at the end of any line,
so no lines with only blanks on them either. Git complains
about such lines when you try to apply a patch with trailing
spaces on lines (and when you create one in the first place, as
well).

You "must" not do anything, you can hope the maintainers will
sort it all out. If you do create a fresh patch, it's best to
not edit a patch file manually, you'll often end up with a
corrupted patch file. "git rebase -i" is your friend; learn
about it if you don't know it yet, it's there to help you :-)

[rogueresearch]

newer patch with fixed whitespace

[rogueresearch]

ok segher, I've attached a new patch with hopefully acceptable whitespace. Can this get merged now? Thanks.

[rogueresearch]

I just wanted to ping this. Can this patch be merged in please?

[stuge]

Replying to rogueresearch:

I just wanted to ping this. Can this patch be merged in please?

Yes, it's in the works. But there's a question:

When exactly does -lobjc need to be included?

1. When linking libusb itself, but never when linking applications against libusb, neither static nor dynamic.
2. When linking libusb itself, and when linking applications statically against libusb.
3. when linking libusb itself, and when linking applications both statically and dynamically against libusb.

This determines exactly where in configure.ac the flag should go.

[rogueresearch]

I'm not sure I really understand the question. My patch adds use of an API, objc_registerThreadWithCollector(), which as we see below comes from libobjc:

$ nm -g /usr/lib/libobjc.dylib | grep objc_registerThreadWithCollector
000000000000c0ae T _objc_registerThreadWithCollector

Thus libusb now requires this lib in the same way that it already requires IOKit.framework. What's the answer to your question for IOKit.framework? The answer would be the same for libobjc I think. My guess is #1.

[rogueresearch]

In [2934cf3604b2e173bc9cb93b873065e776441e1b/libusb-pbatard]:

Darwin: Fix #63 error when apps use Objective-C garbage collection

[rogueresearch]

Confirmed fixed in ​http://git.libusb.org/libusb.git "testing" branch.

[rogueresearch]

In [40327cd134718475f6cec8935b856d4fdff2099c/libusb]:

Darwin: Fix #63 error when apps use Objective-C garbage collection

[www.google.com/accounts/o8/id?id=AItOawkM3uGBTSyyhCOzuNTVLryo9XVnsuR12Vg]

In [d2285744c7fa4007bb411be354268209d350b0f2/libusb]:

configure.ac: Darwin: Move -lobjc from LIBS to PC_LIBS_PRIVATE

Since commit 40327cd134718475f6cec8935b856d4fdff2099c it is neccessary
to explicitly include -lobjc not only when linking libusb itself, but
also for programs linking statically against libusb. References #63.

See also ​http://marc.info/?m=132505900202378