[libbluray-devel] [PATCH 1/2] Get the correct java home on macOS

[Petri Hintukainen]

Ping

Can I apply this patch with following changes:
- BD_DEBUG removed from child process
- exit() changed to _exit() (do we need this ?)

We  need this patch to git. The second patch was already committed, 
and the code doesn't compile without _java_home_macos().

pe, 2016-12-09 kello 15:26 +0200, Petri Hintukainen kirjoitti:
> to, 2016-12-08 kello 03:47 +0100, Marvin Scholz kirjoitti:
> > On macOS apparently the only way to get the correct
> > path to the java home for the currently active SDK as
> > selected by the user is to use the /usr/libexec/java_home
> > tool.
> > 	---
> > It turned out the previous patchset actually still did work fine in
> > all cases, so this one does not use popen/pclose at all anymore,
> > which hopefully works in all cases.
> > 
> > Additionally the check if JLI was loaded properly was done the
> > other
> > way around, so it would log that it was not able to load JLI when
> > it
> > actually was, which is fixed in this version of the second patch.
> > 
> >  src/libbluray/bdj/bdj.c | 89
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 89 insertions(+)
> > 
> > diff --git a/src/libbluray/bdj/bdj.c b/src/libbluray/bdj/bdj.c
> > index d3d4c43..1a03be6 100644
> > --- a/src/libbluray/bdj/bdj.c
> > +++ b/src/libbluray/bdj/bdj.c
> > @@ -39,6 +39,13 @@
> >  #include <stdlib.h>
> >  #include <string.h>
> >  
> > +#ifdef __APPLE__
> > +#include <sys/types.h>
> > +#include <sys/wait.h>
> > +#include <limits.h>
> > +#include <unistd.h>
> > +#endif
> > +
> >  #ifdef _WIN32
> >  #include <windows.h>
> >  #include <winreg.h>
> > @@ -174,6 +181,75 @@ static inline char *_utf8_to_cp(const char
> > *utf8)
> >  }
> >  #endif
> >  
> > +#if defined(__APPLE__) && !defined(HAVE_BDJ_J2ME)
> > +
> > +#define MACOS_JAVA_HOME "/usr/libexec/java_home"
> > +static char *_java_home_macos()
> > +{
> > +    static char result[PATH_MAX] = "";
> > +
> > +    if (result[0])
> > +        return result;
> > +
> > +    pid_t java_home_pid;
> > +    int fd[2], exitcode;
> > +
> > +    if (pipe(fd)) {
> > +        BD_DEBUG(DBG_BDJ | DBG_CRIT, "unable to set up pipes\n");
> > +        return NULL;
> > +    }
> > +
> > +    switch (java_home_pid = vfork())
> 
> I must say I don't like the idea of forking in library. But if there
> are no other ways around the problem ... I don't like running Java VM
> in library either :).
> 
> > +    {
> > +        case -1:
> > +            BD_DEBUG(DBG_BDJ | DBG_CRIT, "vfork failed\n");
> > +            return NULL;
> > +
> > +        case 0:
> > +            if (dup2(fd[1], STDOUT_FILENO) == -1) {
> > +                BD_DEBUG(DBG_BDJ | DBG_CRIT, "unable to dup2
> > fd[1]\n");
> 
> You can't log anything here. It may deadlock or clobber parent
> process
> state. If you want to log child process errors you need to signal the
> error with _exit() and log the message in the parent process.
> 
> > +                exit(-1);
> 
> Should this be _exit() ?
> 
> man vfork:
> "(From POSIX.1) The vfork() function has the same effect as fork(2),
> except that the behavior is undefined if the process created by
> vfork()
> either modifies any data other than a variable of type pid_t used to
> store the return value from vfork(), or returns from the function in
> which vfork() was called, or calls any other function before
> successfully calling _exit(2) or one of the exec(3) family of
> functions."
> 
> > +            }
> > +
> > +            close(fd[1]);
> > +            close(fd[0]);
> > +
> > +            execl(MACOS_JAVA_HOME, MACOS_JAVA_HOME);
> > +
> > +            BD_DEBUG(DBG_BDJ | DBG_CRIT,
> > +                     "unable to execute " MACOS_JAVA_HOME "\n");
> > +
> > +            exit(-1);
> > +
> > +        default:
> > +            close(fd[1]);
> > +
> > +            for (int len = 0; ;) {
> > +                int n = read(fd[0], result + len, sizeof result -
> > len);
> > +                if (n <= 0)
> > +                    break;
> > +
> > +                len += n;
> > +                result[len-1] = '\0';
> > +            }
> > +
> > +            waitpid(java_home_pid, &exitcode, 0);
> > +    }
> > +
> > +    if (result[0] == '\0' || exitcode) {
> > +        BD_DEBUG(DBG_BDJ | DBG_CRIT,
> > +                 "Unable to read path from " MACOS_JAVA_HOME
> > "\n");
> > +        result[0] = '\0';
> > +        return NULL;
> > +    }
> > +
> > +    BD_DEBUG(DBG_BDJ, "macos java home: '%s'\n", result );
> > +    return result;
> > +}
> > +#undef MACOS_JAVA_HOME
> > +
> > +#endif
> > +
> >  static void *_jvm_dlopen(const char *java_home, const char
> > *jvm_dir,
> > const char *jvm_lib)
> >  {
> >      if (java_home) {
> > @@ -210,6 +286,10 @@ static void *_load_jvm(const char
> > **p_java_home)
> >      static const char  jvm_dir[]  = "jre\\bin\\server";
> >      static const char  jvm_lib[]  = "jvm";
> >  # else
> > +#  ifdef __APPLE__
> > +    static const char *jvm_path[] = {NULL, JDK_HOME};
> > +    static const char  jvm_dir[]  = "jre/lib/server";
> > +#  else
> >      static const char *jvm_path[] = {NULL, JDK_HOME,
> >                                       "/usr/lib/jvm/default-java",
> >                                       "/usr/lib/jvm/default",
> > @@ -220,6 +300,7 @@ static void *_load_jvm(const char
> > **p_java_home)
> >                                       "/usr/lib/jvm/java-6-
> > openjdk",
> >      };
> >      static const char  jvm_dir[]  = "jre/lib/" JAVA_ARCH
> > "/server";
> > +#  endif
> >      static const char  jvm_lib[]  = "libjvm";
> >  # endif
> >  #endif
> > @@ -241,6 +322,14 @@ static void *_load_jvm(const char
> > **p_java_home)
> >      }
> >  #endif
> >  
> > +#if defined(__APPLE__) && !defined(HAVE_BDJ_J2ME)
> > +    java_home = _java_home_macos();
> > +    if (java_home) {
> > +        *p_java_home = java_home;
> > +        return _jvm_dlopen(java_home, jvm_dir, jvm_lib);
> > +    }
> > +#endif
> > +
> >      BD_DEBUG(DBG_BDJ, "JAVA_HOME not set, trying default
> > locations\n");
> >  
> >      /* try our pre-defined locations */