> On July 5, 2016, 9:22 a.m., Jonathan Hurley wrote: > > I don't see how this actually fixes the problem; importing gc from > > subprocess would be the same as importing it normally, no? There isn't a > > different gc module that subprocess uses. Also, after you patch it, you > > then re-import it - wouldn't that break the existing patching? > > Andrew Onischuk wrote: > "I don't see how this actually fixes the problem" - this was tested on > the instance where I could reproduce this and is a sure fix.
So, let's say that it does fix the issue by always reporting that GC is enabled. Wouldn't this then cause side effects since Popen is specifically disabling it for a reason? > On July 5, 2016, 9:22 a.m., Jonathan Hurley wrote: > > ambari-agent/src/main/python/ambari_agent/main.py, lines 33-36 > > <https://reviews.apache.org/r/49535/diff/1/?file=1435405#file1435405line33> > > > > Could you just do a reload(gc) here? > > Andrew Onischuk wrote: > reload python module does more complicated things than this, here we > simply need to reimport existing compiled module. OK, but you re-imported the same compiled module - doesn't that overwrite your previous patching? - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49535/#review140761 ----------------------------------------------------------- On July 4, 2016, 4:45 a.m., Andrew Onischuk wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49535/ > ----------------------------------------------------------- > > (Updated July 4, 2016, 4:45 a.m.) > > > Review request for Ambari and Dmitro Lisnichenko. > > > Bugs: AMBARI-17539 > https://issues.apache.org/jira/browse/AMBARI-17539 > > > Repository: ambari > > > Description > ------- > > *Reason of memory leak:* > Race condition in subprocess python module. > Due to this race condition at some unlucky cases python garbage collection > was disabled. > This usually happened when running alerts, as a bunch of our alerts run shell > commands and they do it in a different threads. > > *Fix for the issue:* > Synchronizing subprocess is not the best option. Since some people can still > use it without synchronization not knowing about the issue. > Also synchronizing will provide some unnecessary slowdown. So for this issue > the proposed fix is to monkey patch subprocess.gc.isenabled. > > > Diffs > ----- > > ambari-agent/src/main/python/ambari_agent/main.py 4db89f8 > > Diff: https://reviews.apache.org/r/49535/diff/ > > > Testing > ------- > > mvn clean test > > > Thanks, > > Andrew Onischuk > >
