> On Jan. 4, 2016, 8:25 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/thermos/observer/task_observer.py, line 72 > > <https://reviews.apache.org/r/41915/diff/1/?file=1181509#file1181509line72> > > > > We usually put this default initialization directly into the arg list, > > e.g:'stop_event=threading.Event()'.
I'm leery of that for mutable objects like an Event. Surprising things happen if/when the containing object gets constructed a 2nd time and the single default Event has been mutated! - John ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41915/#review112750 ----------------------------------------------------------- On Jan. 4, 2016, 8:02 p.m., John Sirois wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41915/ > ----------------------------------------------------------- > > (Updated Jan. 4, 2016, 8:02 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Bugs: AURORA-1570 > https://issues.apache.org/jira/browse/AURORA-1570 > > > Repository: aurora > > > Description > ------- > > Previously, a mock threading.Event was waited on in one thread > and the count of waits was read in another thread. Most thread > memory models do not guaranty reads are fresh in this scenario > unless there is a memory barrier of some sort forcing per-cpu > caches to be flushed. > > This change uses the underlying threading.Event as the memory > barrier instead of mocking it and just wraps the event to record > calls manually. > > src/main/python/apache/thermos/observer/task_observer.py | 5 +++-- > src/test/python/apache/thermos/observer/test_task_observer.py | 36 > ++++++++++++++++++++++++------------ > 2 files changed, 27 insertions(+), 14 deletions(-) > > > Diffs > ----- > > src/main/python/apache/thermos/observer/task_observer.py > 1485de8faef52716f11b82a3556064de26c67427 > src/test/python/apache/thermos/observer/test_task_observer.py > ace15c5305e75fac3a82971f4d71b92bcb37bafc > > Diff: https://reviews.apache.org/r/41915/diff/ > > > Testing > ------- > > Before this change I got a failure between 1/5 and 1/10th of the > time via: > ``` > while true > do > ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest > done > ``` > > After the change I cannot trigger the failure. > > > Thanks, > > John Sirois > >
