Mac: hid_enumerate crash on macOS 14 when linked to AppKit and inside a `fork` #587

New issue

Closed

opened 2023-06-09 12:34:46 -05:00 by nguyen-phillip · 5 comments

nguyen-phillip commented

2023-06-09 12:34:46 -05:00

(Migrated from github.com)

However it seems that the only reason why the hidapi code needs to be linked against AppKit is because it references NSAppKitVersionNumber to check for if running on macOS >= 10.10 Yosemite

github.com/libusb/hidapi@80e8d62bc1/mac/hid.c (L469)

Would it be reasonable to send a pull request that dropped support for macOS < 10.10? This would simplify the code a bit and remove the requirement for linking with AppKit entirely.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <hidapi.h>

int main(int argc, char **argv) {
  if (argc > 1 && !strcmp(argv[1], "--fork")) {
    pid_t pid = fork();
    printf("fork: %d\n", pid);
    if (pid != 0) {      
      printf("parent exiting\n");
      exit(0);
    }
  }  
  struct hid_device_info *info = hid_enumerate(0, 0);
  printf("hid_device_info: %lx\n", (long)info);
  return 0;
}

The example code below crashes in macOS 14 on a call to IOHIDManagerCreate when linked against AppKit and the --fork flag is passed. Removing the AppKit dependency allows it to run as expected. I've filed a case with Apple since this seems to be an OS bug. However it seems that the only reason why the hidapi code needs to be linked against AppKit is because it references NSAppKitVersionNumber to check for if running on macOS >= 10.10 Yosemite https://github.com/libusb/hidapi/blob/80e8d62bc1cc70876d57fed7e76d7478757b4da8/mac/hid.c#L469 Would it be reasonable to send a pull request that dropped support for macOS < 10.10? This would simplify the code a bit and remove the requirement for linking with AppKit entirely. ``` #include <stdio.h> #include <stdlib.h> #include <string.h> #include <unistd.h> #include <hidapi.h> int main(int argc, char **argv) { if (argc > 1 && !strcmp(argv[1], "--fork")) { pid_t pid = fork(); printf("fork: %d\n", pid); if (pid != 0) { printf("parent exiting\n"); exit(0); } } struct hid_device_info *info = hid_enumerate(0, 0); printf("hid_device_info: %lx\n", (long)info); return 0; } ```

Youw commented

2023-06-09 13:17:13 -05:00

(Migrated from github.com)

I don't think I have enough info to tell if we should drop the 10.10 support

can we instead have compile-time desision based on the -mmacosx-version-min compilation flag (I'm not sure if there is corresponding compile-time macro definition - I presume there is)?
that should be both safe and backward compatible

I don't think I have enough info to tell if we should drop the 10.10 support can we instead have compile-time desision based on the `-mmacosx-version-min` compilation flag (I'm not sure if there is corresponding compile-time macro definition - I presume there is)? that should be both safe and backward compatible

nguyen-phillip commented

2023-06-09 14:55:23 -05:00

(Migrated from github.com)

Maybe something like

is_macos_10_10_or_greater = MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_10;

would work?

Maybe something like ``` is_macos_10_10_or_greater = MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_10; ``` would work?

Youw commented

2023-06-09 15:51:42 -05:00

(Migrated from github.com)

Unfortunately no. It has to be more like:

is_macos_10_10_or_greater = 
#if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_10
    true;
#else
    (NSAppKitVersionNumber >= 1343);
#endif

and the declaration of NSAppKitVersionNumber has to be under similar ifdef
so is the link library in CMakeList.txt

Unfortunately no. It has to be more like: ```c is_macos_10_10_or_greater = #if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_10 true; #else (NSAppKitVersionNumber >= 1343); #endif ``` and the declaration of `NSAppKitVersionNumber` has to be under similar `ifdef` so is the link library in `CMakeList.txt`

nguyen-phillip commented

2023-06-09 22:03:47 -05:00

(Migrated from github.com)

Yes, that's less than ideal. Another option suggested to me was to use uname to get the Darwin kernel version, which for macOS >= 10.10 would be Darwin >= 14.0.0. Comparing the version strings properly then means extracting out the first dot component of the version string and converting it to an integer. This seems like a lot of extra code just to avoid linking AppKit. I think if continued support for pre 10.10 versions is desired, it's probably fine to close this as infeasible.

Yes, that's less than ideal. Another option suggested to me was to use `uname` to get the Darwin kernel version, which for macOS >= 10.10 would be Darwin >= 14.0.0. Comparing the version strings properly then means extracting out the first dot component of the version string and converting it to an integer. This seems like a lot of extra code just to avoid linking AppKit. I think if continued support for pre 10.10 versions is desired, it's probably fine to close this as infeasible.

Youw commented

2023-06-10 12:03:53 -05:00

(Migrated from github.com)

that's less than ideal

And what I suggest and will gladely accept if anyone opens a PR.

Comparing the version strings

Yeah, I prefer avoiding such if possiblle.

> that's less than ideal And what I suggest and will gladely accept if anyone opens a PR. > Comparing the version strings Yeah, I prefer avoiding such if possiblle.