facebookarchive / libphenom

An eventing framework for building high performance and high scalability systems in C.
http://facebook.github.io/libphenom
Apache License 2.0
1.66k stars 362 forks source link

Can't call ph_sched_run() after ph_sched_stop() #51

Closed brucespang closed 10 years ago

brucespang commented 10 years ago

After being stopped with ph_sched_stop(), ph_sched_run() tears down the emitters structure, which it didn't create (https://github.com/facebook/libphenom/blob/e486ef426f1af668869900e982d13d314b45cccb/corelib/nbio/common.c#L399). If we try to run ph_sched_run() again, emitters is null so it segfaults.

We can't run ph_nbio_init() to fix this, because counter_scope is still around from the first call (https://github.com/facebook/libphenom/blob/e486ef426f1af668869900e982d13d314b45cccb/corelib/nbio/common.c#L249).

This makes it hard to use libphenom for e.g. DNS resolution in tests.

wez commented 10 years ago

I'm not 100% sold on stopping and re-starting the scheduler; unlike other eventing libraries, phenom has more of a global view of the world with its scheduler guts and assumes that the scheduler will be running for the life of the program. Other libraries don't manage so holistically and need to provide more explicit control of the scheduler loop.

There are pro's and con's to both approaches.

Per my comments in #52, I don't think this architecture gets in the way of testing but I'm likely missing something about your approach that we should discuss :)

brucespang commented 10 years ago

Seems reasonable. I can add some documentation/assertions that make it clear that restarting the scheduler is not possible.

I did figure out how to yet things properly though. Running the scheduler in one thread and tests in another seems to work very well.

On Feb 15, 2014, at 5:28 PM, Wez Furlong notifications@github.com wrote:

I'm not 100% sold on stopping and re-starting the scheduler; unlike other eventing libraries, phenom has more of a global view of the world with its scheduler guts and assumes that the scheduler will be running for the life of the program. Other libraries don't manage so holistically and need to provide more explicit control of the scheduler loop.

There are pro's and con's to both approaches.

Per my comments in #52, I don't think this architecture gets in the way of testing but I'm likely missing something about your approach that we should discuss :)

— Reply to this email directly or view it on GitHub.

wez commented 10 years ago

I think we're good here too