search

secondlife / viewer

🖥️ Second Life's official client
GNU Lesser General Public License v2.1
212 stars 53 forks source link

Give control over thread queue's automatic shutdown #2982

Closed akleshchev closed 3 weeks ago

akleshchev commented 3 weeks ago

queue can shutdown before parent thread is done

akleshchev commented 3 weeks ago

My question is: now that WorkQueueBase has auto_shutdown, why don't we remove that feature from ThreadPoolBase?

@nat-goodspeed Are you suggesting to remove mStopListener from ThreadPoolBase? To me it feels like thread should be one in control, queue shouldn't die on it out the blue. Arguably I should be calling new queue_t(name, capacity, false) to let thread decide when it's ready to die.

nat-goodspeed commented 3 weeks ago

Hmm, now that I'm looking at ThreadPoolBase::close(), I see that it does more than call WorkQueueBase::close(): it also joins the ThreadPoolBase threads. Sorry, my previous suggestion was ill-considered.

So this PR is about the case when we have a WorkQueue or WorkSchedule, not associated with a ThreadPool, that shouldn't close() when LLCoros signals the start of viewer shutdown? Maybe it should instead listen for a later value of "status" ?

akleshchev commented 3 weeks ago

Maybe it should instead listen for a later value of "status" ?

This is for the sake of window thread which under normal circumstances needs to keep living (and displaying shutting down progress bar) after event system dies and then main thread needs to post a proper shutdown/destroy window event to window thread's queue.

nat-goodspeed commented 3 weeks ago

Okay, I see. In that case I agree with your earlier suggestion.

Arguably I should be calling new queue_t(name, capacity, false) to let thread decide when it's ready to die.

If both ThreadPoolBase and WorkQueueBase see auto_shutdown=true, they register redundant listeners.

akleshchev commented 3 weeks ago

Updated with new queue_t(name, capacity, false).