----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23216/#review47678 -----------------------------------------------------------
src/examples/low_level_scheduler_pthread.cpp <https://reviews.apache.org/r/23216/#comment83812> What about combining these two lines? src/examples/low_level_scheduler_pthread.cpp <https://reviews.apache.org/r/23216/#comment83807> A subtle race condition here: 1) the control reaches right before pthread_cond_wait(...); 2) connected callback is called, and pthread_cond_signal(..) is called. 3) pthread_cond_wait(...) is called. Looks like a deadlock bug to me. src/examples/low_level_scheduler_pthread.cpp <https://reviews.apache.org/r/23216/#comment83808> Why sleep(1) here? src/examples/low_level_scheduler_pthread.cpp <https://reviews.apache.org/r/23216/#comment83814> Ditto. src/examples/low_level_scheduler_pthread.cpp <https://reviews.apache.org/r/23216/#comment83815> Ditto. src/examples/low_level_scheduler_pthread.cpp <https://reviews.apache.org/r/23216/#comment83813> No need to protect framework. src/examples/low_level_scheduler_pthread.cpp <https://reviews.apache.org/r/23216/#comment83810> call.set_type(state == INITIALIZING ? Call::REGISTER : Call::REREGISTER); src/examples/low_level_scheduler_pthread.cpp <https://reviews.apache.org/r/23216/#comment83811> You don't really need to protect 'framework' here. Although all callbacks are executed using async(), but they are guarded by mutex.lock in src/scheduler/scheduler.cpp, so all the callbacks will be effectively executed serially. If 'framework' is only accessed by these callbacks, you don't need a lock. src/examples/low_level_scheduler_pthread.cpp <https://reviews.apache.org/r/23216/#comment83803> Use path::join() instead? src/examples/low_level_scheduler_pthread.cpp <https://reviews.apache.org/r/23216/#comment83805> Use EXIT(1) just to be consistent? src/examples/low_level_scheduler_pthread.cpp <https://reviews.apache.org/r/23216/#comment83806> Ditto. src/examples/low_level_scheduler_pthread.cpp <https://reviews.apache.org/r/23216/#comment83804> Rename it to wait()? - Jie Yu On July 11, 2014, 8:55 p.m., Zuyu Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23216/ > ----------------------------------------------------------- > > (Updated July 11, 2014, 8:55 p.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, Jie Yu, and Vinod > Kone. > > > Repository: mesos-git > > > Description > ------- > > Added the low level scheduler example using pthread. > > > Diffs > ----- > > src/Makefile.am 45afcd145f3b502043424a6dac2197979aefbca2 > src/examples/low_level_scheduler_pthread.cpp PRE-CREATION > src/tests/examples_tests.cpp 2b554d72f0058a68f589719373f3d3e37a3a7ba3 > src/tests/low_level_scheduler_pthread_test.sh PRE-CREATION > > Diff: https://reviews.apache.org/r/23216/diff/ > > > Testing > ------- > > [==========] Running 1 test from 1 test case. > [----------] Global test environment set-up. > [----------] 1 test from ExamplesTest > [ RUN ] ExamplesTest.LowLevelSchedulerPthread > [ OK ] ExamplesTest.LowLevelSchedulerPthread (1655 ms) > [----------] 1 test from ExamplesTest (1655 ms total) > > [----------] Global test environment tear-down > [==========] 1 test from 1 test case ran. (1669 ms total) > [ PASSED ] 1 test. > > > Thanks, > > Zuyu Zhang > >
