This is the mail archive of the
java-patches@gcc.gnu.org
mailing list for the Java project.
Patch: PR libgcj/23758, Fix theoretical race PosixProcess.
- From: David Daney <ddaney at avtrex dot com>
- To: GCJ-patches <java-patches at gcc dot gnu dot org>
- Date: Sun, 08 Apr 2007 22:48:13 -0700
- Subject: Patch: PR libgcj/23758, Fix theoretical race PosixProcess.
Thanks to Hans Boehm for noticing this problem.
In a multi-threaded program (like libgcj), malloc may need to acquire a
mutex. If a thread forks while the hypothetical mutex is held (by a
different thread), it will block forever if it invokes malloc because
the mutex will never be released.
This patch moves the mallocs out of the danger zone in natPosixProcess.cc
Tested on x86_64-pc-linux-gnu with no failures in make -k check in libjava.
OK to commit?
2007-04-08 David Daney <ddaney@avtrex.com>
PR libgcj/23758
* java/lang/natPosixProcess.cc (nativeSpawn): Move building of
environment before the fork.
* testsuite/libjava.lang/Process_7.java: New test.
* testsuite/libjava.lang/Process_7.out: Its expected results.
* testsuite/libjava.lang/Process_7.jar: Generated file.
Index: testsuite/libjava.lang/Process_7.out
===================================================================
--- testsuite/libjava.lang/Process_7.out (revision 0)
+++ testsuite/libjava.lang/Process_7.out (revision 0)
@@ -0,0 +1 @@
+ok
Index: testsuite/libjava.lang/Process_7.jar
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: testsuite/libjava.lang/Process_7.jar
___________________________________________________________________
Name: svn:mime-type
+ application/octet-stream
Index: testsuite/libjava.lang/Process_7.java
===================================================================
--- testsuite/libjava.lang/Process_7.java (revision 0)
+++ testsuite/libjava.lang/Process_7.java (revision 0)
@@ -0,0 +1,45 @@
+// Verify we can modify the environment.
+import java.io.BufferedReader;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.util.Map;
+
+
+
+public class Process_7
+{
+ public static void main(String[] args)
+ {
+ try
+ {
+ ProcessBuilder pb = new ProcessBuilder("env");
+ Map<String, String> e = pb.environment();
+ e.clear();
+ String v = "process7_value";
+ String k = "PROCESS_7_KEY";
+ e.put(k, v);
+ Process p = pb.start();
+ InputStream is = p.getInputStream();
+ InputStreamReader isr = new InputStreamReader(is);
+ BufferedReader br = new BufferedReader(isr);
+ boolean found = false;
+
+ String result;
+ while ((result = br.readLine()) != null)
+ {
+ if (result.equals(k + '=' + v))
+ found = true;
+ }
+ if (!found)
+ {
+ System.out.println("bad");
+ return;
+ }
+ System.out.println("ok");
+ }
+ catch (Exception ex)
+ {
+ System.out.println(ex.toString());
+ }
+ }
+}
Index: java/lang/natPosixProcess.cc
===================================================================
--- java/lang/natPosixProcess.cc (revision 123656)
+++ java/lang/natPosixProcess.cc (working copy)
@@ -1,6 +1,7 @@
// natPosixProcess.cc - Native side of POSIX process code.
-/* Copyright (C) 1998, 1999, 2000, 2002, 2003, 2004, 2005, 2006, 2007 Free Software Foundation
+/* Copyright (C) 1998, 1999, 2000, 2002, 2003, 2004, 2005, 2006, 2007
+ Free Software Foundation
This file is part of libgcj.
@@ -248,16 +249,57 @@ java::lang::PosixProcess::nativeSpawn ()
if (envp)
{
- env = (char **) _Jv_Malloc ((envp->length + 1) * sizeof (char *));
+ bool need_path = true;
+ bool need_ld_library_path = true;
+ int i;
+
+ // Preserve PATH and LD_LIBRARY_PATH unless specified
+ // explicitly. We need three extra slots. Potentailly PATH
+ // and LD_LIBRARY_PATH will be added plus the NULL
+ // termination.
+ env = (char **) _Jv_Malloc ((envp->length + 3) * sizeof (char *));
elts = elements (envp);
// Initialize so we can gracefully recover.
- for (int i = 0; i <= envp->length; ++i)
+ for (i = 0; i < envp->length + 3; ++i)
env[i] = NULL;
- for (int i = 0; i < envp->length; ++i)
- env[i] = new_string (elts[i]);
- env[envp->length] = NULL;
+ for (i = 0; i < envp->length; ++i)
+ {
+ env[i] = new_string (elts[i]);
+ if (0 == strncmp (env[i], "PATH=", sizeof("PATH=")))
+ need_path = false;
+ if (0 == strncmp (env[i], "LD_LIBRARY_PATH=",
+ sizeof("LD_LIBRARY_PATH=")))
+ need_ld_library_path = false;
+ }
+
+ if (need_path)
+ {
+ char *path_val = getenv ("PATH");
+ if (path_val)
+ {
+ env[i] = (char *) _Jv_Malloc (strlen (path_val) +
+ sizeof("PATH=") + 1);
+ strcpy (env[i], "PATH=");
+ strcat (env[i], path_val);
+ i++;
+ }
+ }
+ if (need_ld_library_path)
+ {
+ char *path_val = getenv ("LD_LIBRARY_PATH");
+ if (path_val)
+ {
+ env[i] =
+ (char *) _Jv_Malloc (strlen (path_val) +
+ sizeof("LD_LIBRARY_PATH=") + 1);
+ strcpy (env[i], "LD_LIBRARY_PATH=");
+ strcat (env[i], path_val);
+ i++;
+ }
+ }
+ env[i] = NULL;
}
// We allocate this here because we can't call malloc() after
@@ -303,29 +345,7 @@ java::lang::PosixProcess::nativeSpawn ()
{
// Child process, so remap descriptors, chdir and exec.
if (envp)
- {
- // Preserve PATH and LD_LIBRARY_PATH unless specified
- // explicitly.
- char *path_val = getenv ("PATH");
- char *ld_path_val = getenv ("LD_LIBRARY_PATH");
- environ = env;
- if (path_val && getenv ("PATH") == NULL)
- {
- char *path_env =
- (char *) _Jv_Malloc (strlen (path_val) + 5 + 1);
- strcpy (path_env, "PATH=");
- strcat (path_env, path_val);
- putenv (path_env);
- }
- if (ld_path_val && getenv ("LD_LIBRARY_PATH") == NULL)
- {
- char *ld_path_env =
- (char *) _Jv_Malloc (strlen (ld_path_val) + 16 + 1);
- strcpy (ld_path_env, "LD_LIBRARY_PATH=");
- strcat (ld_path_env, ld_path_val);
- putenv (ld_path_env);
- }
- }
+ environ = env;
// We ignore errors from dup2 because they should never occur.
dup2 (outp[0], 0);
@@ -344,7 +364,7 @@ java::lang::PosixProcess::nativeSpawn ()
close (outp[0]);
close (outp[1]);
close (msgp[0]);
-
+
// Change directory.
if (path != NULL)
{