----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15333/#review29028 -----------------------------------------------------------
3rdparty/libprocess/include/process/owned.hpp <https://reviews.apache.org/r/15333/#comment56113> IIUC, you're using 'shared' to protect against two threads/processes from simultaneously calling Owned::share? Why would two threads/processes be doing this in the first place (considering this is supposed to be "owned")? Even if this was the case, I'm not convinced you've got sufficient coverage. Consider: thread 1 context switches before the CAS in Owned::share (line 143) and thread 2 executes Owned::get and context switches before 'return data->t;' (line 106) then thread 1 resumes and does 'data.reset()' which means thread 2 is going to deference a NULL pointer when it resumes. Unless I'm missing something? 3rdparty/libprocess/include/process/shared.hpp <https://reviews.apache.org/r/15333/#comment56112> How about: "Transfers ownership of the pointer by waiting for exclusive access (i.e., no other Shared instances). ..." 3rdparty/libprocess/include/process/shared.hpp <https://reviews.apache.org/r/15333/#comment56111> How about: "Ownership is already being transferred". - Benjamin Hindman On Nov. 8, 2013, 1:30 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15333/ > ----------------------------------------------------------- > > (Updated Nov. 8, 2013, 1:30 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > See summary. > > Renamed to Shared<T>::upgrade to Shared<T>::own(). > > Also, moved owned.hpp to libprocess. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/Makefile.am 47e8dbb > 3rdparty/libprocess/3rdparty/stout/include/stout/owned.hpp 3433f50 > 3rdparty/libprocess/Makefile.am d348780 > 3rdparty/libprocess/include/process/owned.hpp PRE-CREATION > 3rdparty/libprocess/include/process/shared.hpp e0c7991 > 3rdparty/libprocess/include/process/statistics.hpp fbae641 > 3rdparty/libprocess/src/tests/owned_tests.cpp PRE-CREATION > 3rdparty/libprocess/src/tests/shared_tests.cpp 860a9aa > 3rdparty/libprocess/src/tests/statistics_tests.cpp 8695f45 > src/master/master.hpp e377af8 > src/master/master.cpp 8e14a07 > src/sched/sched.cpp 3abe72f > src/slave/gc.hpp 083aa79 > src/slave/monitor.cpp 9cb6256 > src/slave/reaper.hpp 4498139 > src/slave/slave.hpp 68526f3 > src/tests/cluster.hpp bea395a > > Diff: https://reviews.apache.org/r/15333/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
