Code review comment for lp:~ralph-lange/epics-base/thread-hooks

Revision history for this message
Ralph Lange (ralph-lange) wrote :

> Please don't mark new API routines with epicsShareAPI; I'm trying to phase
> that out [...]

Will do.

> This may be more of a discussion point. Here's my output from
> epicsThreadShowAll:
>
> epics> epicsThreadShowAll
> NAME EPICS ID LWP ID OSIPRI OSSPRI STATE
> _main_ 0x10b1060 0 0 0 OK
> errlog 0x113c830 20849 10 0 OK
> [...]
>
> The _main_ thread is not started using epicsThreadCreate() and thus never runs
> the thread_hook, so we don't know its LWP ID. It would be nice if we could
> see it here.

I had considered this, then decided against, as the POSIX implementation shows the POSIX thread id as 0, and I wanted to stay minimally invasive.
If you think this is a good idea (I do), I will add that.

> I think we can safely forget about calling _main_'s ExitHook, which brings up
> the interesting question of whether ExitHook is actually needed -- any
> StartHook can register an epicsAtThreadExit() routine if it needs to do clean-
> up. If we were to remove the ExitHooks we would never have problems with an
> ExitHook being called for a thread for which where there was no corresponding
> StartHook.

Good! That was my initial approach, and I added the ExitHook routines only out of a gut feeling that I wold have to justify if I didn't.
The argument about an exit hook being called where the start hook wasn't added at thread creation time is an excellent one.

> It also means that the ExitHook routines would be run in the
> reverse order of the StartHook routines, whereas at the moment the order is
> the same.

Not true. They are run in correct (reverse) order, which is covered by the test. Never mind.

~Ralph

« Back to merge proposal