On 30/11/16 17:53, Chris Kaynor wrote: > On Wed, Nov 30, 2016 at 9:34 AM, duncan smith <duncan@invalid.invalid> wrote: >> Hello, >> I have had an issue with some code for a while now, and I have not >> been able to solve it. I use the subprocess module to invoke dot >> (Graphviz) to generate a file. But if I do this repeatedly I end up with >> an error. The following traceback is from a larger application, but it >> appears to be repeated calls to 'to_image' that is the issue. > > I don't see any glaring problems that would obviously cause this, > however have you checked to see if the processes are actually exiting > (it looks like you are on Linux, so the top command)? > >> >> >> Traceback (most recent call last): >> File "<pyshell#80>", line 1, in <module> >> z = link_exp.sim1((djt, tables), variables, 1000, 400, 600, >> [0,1,2,3,4,5,6], [6,7,8,9,10], ind_gens=[link_exp.males_gen()], >> ind_gens_names=['Forename'], seed='duncan') >> File "link_exp.py", line 469, in sim1 >> RL_F2 = EM_setup(data) >> File "link_exp.py", line 712, in full_EM >> last_g = prop.djt.g >> File "Nin.py", line 848, in draw_model >> dot_g.to_image(filename, prog='dot', format=format) >> File "dot.py", line 597, in to_image >> to_image(str(self), filename, prog, format) >> File "dot.py", line 921, in to_image >> _execute('%s -T%s -o %s' % (prog, format, filename)) >> File "dot.py", line 887, in _execute >> close_fds=True) >> File "/usr/lib/python2.7/subprocess.py", line 711, in __init__ >> errread, errwrite) >> File "/usr/lib/python2.7/subprocess.py", line 1235, in _execute_child >> self.pid = os.fork() >> OSError: [Errno 12] Cannot allocate memory >> >> >> The relevant (AFAICT) code is, >> >> >> def to_image(text, filename, prog='dot', format='dot'): >> # prog can be a series of commands >> # like 'unflatten -l 3 | dot' >> handle, temp_path = tempfile.mkstemp() >> f = open(temp_path, 'w') >> try: >> f.write(text) >> f.close() >> progs = prog.split('|') >> progs[0] = progs[0] + ' %s ' % temp_path >> prog = '|'.join(progs) >> _execute('%s -T%s -o %s' % (prog, format, filename)) >> finally: >> f.close() >> os.remove(temp_path) >> os.close(handle) >> >> def _execute(command): >> # shell=True security hazard? >> p = subprocess.Popen(command, shell=True, stdin=subprocess.PIPE, >> stdout=subprocess.PIPE, >> stderr=subprocess.STDOUT, >> close_fds=True) >> output = p.stdout.read() >> p.stdin.close() >> p.stdout.close() >> #p.communicate() >> if output: >> print output > > This code has a potential dead-lock. If you are calling it from > multiple threads/processes, it could cause issues. This should be > obvious, as your program will also not exit. The communicate call is > safe, but commented out (you'd need to remove the three lines above it > as well). Additionally, you could just set stdin=None rather than > PIPE, which avoids the dead-lock, and you aren't using stdin anyways. > This issues comes if the subprocess may ever wait for something to be > written to stdin, it will block forever, but your call to read will > also block until it closes stdout (or possibly other cases). Another > option would be to close stdin before starting the read, however if > you ever write to stdin, you'll reintroduce the same issue, depending > on OS buffer sizes. > > My question above also comes from the fact that I am not 100% sure > when stdout.read() will return. It is possible that a null or EOF > could cause it to return before the process actually exits. The > subprocess could also expliciting close its stdout, causing it to > return while the process is still running. I'd recommend adding a > p.wait() or just uncommenting the p.communicate() call to avoid these > issues. > > Another, unrelated note, the security hazard depends on where the > arguments to execute are coming from. If any of those are controlled > from untrusted sources (namely, user input), you have a > shell-injection attack. Imagine, for example, if the user requests the > filename "a.jpg|wipehd" (note: I don't know the format command on > Linux, so replace with your desired command). This will cause your > code to wipe the HD by piping into the command. If all of the inputs > are 100% sanitized or come from trusted sources, you're fine, however > that can be extremely difficult to guarantee. >
Thanks. So something like the following might do the job? def _execute(command): p = subprocess.Popen(command, shell=False, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, close_fds=True) out_data, err_data = p.communicate() if err_data: print err_data Duncan -- https://mail.python.org/mailman/listinfo/python-list