Thread: Rename some signal and interrupt handling functions for consistency

The usual pattern for handling a signal is that the signal handler sets 
a global variable and calls SetLatch(MyLatch) to wake up the process, 
and later CHECK_FOR_INTERRUPTS() or other code that is part of a wait 
loop calls another function to deal with it. The naming of the functions 
involved was a bit inconsistent, however. CHECK_FOR_INTERRUPTS() calls 
ProcessInterrupts() to do the heavy-lifting, and the subroutines of 
ProcessInterrupts() were called Process*(), except for 
HandleParallelMessages() and HandleParallelApplyMessages().

Some aux processes don't call CHECK_FOR_INTERRUPTS() and 
ProcessInterrupts() in the main loop, but they have analogous functions 
like HandleMainLoopInterrupts(), HandleStartupProcInterrupts(), etc.

To make things less confusing, the attached patch renames all the 
functions that are part of the overall signal/interrupt handling system 
but are *not* executed in a signal handler to e.g. ProcessSomething(), 
rather than HandleSomething().

Any objections?

P.S. I bumped into this during the larger interrupt revamping work, see 
"Interrupts vs signals" thread. With that, most of the remaining 
functions that are now called Handle*() will go away, and the Process*() 
functions will mostly stay. But IMHO this makes sense independently of 
that work.

-- 
Heikki Linnakangas
Neon (https://neon.tech)

On Tue, Mar 04, 2025 at 08:22:02PM +0200, Heikki Linnakangas wrote:
> To make things less confusing, the attached patch renames all the functions
> that are part of the overall signal/interrupt handling system but are *not*
> executed in a signal handler to e.g. ProcessSomething(), rather than
> HandleSomething().

Am I understanding correctly that your plan is to keep the "Handle" prefix
for functions that do run in signal handlers (e.g.,
HandleRecoveryConflictInterrupt())?  I don't know how consistent the code
is about that, but it might be nice to establish stricter guidelines for
those, too.

> Any objections?

No objections here.  My only concern is that this might break some
third-party code, especially code that uses interrupt.h.  I'm not sure it's
worth adding backward-compatibility macros, though.

-- 
nathan

On 04/03/2025 21:38, Nathan Bossart wrote:
> On Tue, Mar 04, 2025 at 08:22:02PM +0200, Heikki Linnakangas wrote:
>> To make things less confusing, the attached patch renames all the functions
>> that are part of the overall signal/interrupt handling system but are *not*
>> executed in a signal handler to e.g. ProcessSomething(), rather than
>> HandleSomething().
> 
> Am I understanding correctly that your plan is to keep the "Handle" prefix
> for functions that do run in signal handlers (e.g.,
> HandleRecoveryConflictInterrupt())?  I don't know how consistent the code
> is about that, but it might be nice to establish stricter guidelines for
> those, too.

Correct.

There are some completely unrelated functions that also have "Handle" 
prefix, though, like HandleUploadManifestPacket() and 
HandleFunctionRequest(). Ignoring those, all the remaining functions 
that are in any way related to signal / interrupt handling and have the 
"Handle" prefix run in signal handlers.

>> Any objections?
> 
> No objections here.  My only concern is that this might break some
> third-party code, especially code that uses interrupt.h.  I'm not sure it's
> worth adding backward-compatibility macros, though.

I don't think it's worth fretting over. The only one of these functions 
that extensions should be calling is ProcessMainLoopInterrupts(), and an 
extension can easily add an compatibility #ifdef for that if they want to.

Thanks!

-- 
Heikki Linnakangas
Neon (https://neon.tech)