> On May 21, 2014, 6:09 a.m., Benjamin Hindman wrote: > > 3rdparty/libprocess/include/process/owned.hpp, line 148 > > <https://reviews.apache.org/r/21721/diff/2/?file=585621#file585621line148> > > > > Not your code, but the semantics of this CHECK are a bit weird and > > since you're copying the semantics below I think we should clean this up. > > In particular, if 'data.get()' is NULL in the if check above then we return > > Shared<T>(NULL), but if we lose a race when we do __sync_fetch_and_and then > > we have a CHECK failure! Said another way, if we lose the race on line 147 > > we get a CHECK failure, but if we lose the race just after line 150 we > > return Shared<T>(NULL). Seems like we should always return Shared<T>(NULL).
Agreed with Ben. Let's clean it up. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21721/#review43543 ----------------------------------------------------------- On May 20, 2014, 8:40 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21721/ > ----------------------------------------------------------- > > (Updated May 20, 2014, 8:40 p.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Jie Yu. > > > Repository: mesos-git > > > Description > ------- > > Will be used in Authorizer. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/owned.hpp > aacd7b87c29fd31767621c475df98c819283cff9 > 3rdparty/libprocess/src/tests/owned_tests.cpp > 234469b5e549fc7345ee2912c32c63aa3bb95a13 > > Diff: https://reviews.apache.org/r/21721/diff/ > > > Testing > ------- > > libprocess make check > > > Thanks, > > Vinod Kone > >
