Skip to content

Initial support for engines library #2367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yadij wants to merge 6 commits into
base: master
Choose a base branch
from
+191 −199
Open

Initial support for engines library #2367

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b26a5ec
Initial libengines.la structure
yadij Feb 4, 2026
f07c21a
Move AsyncEngine to libengines
yadij Feb 4, 2026
9e573fb
Polish AsyncEngine documentation and source format
yadij Feb 4, 2026
a789f28
Reviewer requested forward.h
yadij Feb 6, 2026
2243c0a
Move EventLoop to libengines.la
yadij Feb 7, 2026
a634410
Polish code style of EventLoop
yadij Feb 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2578,6 +2578,7 @@ AC_CONFIG_FILES([
src/DiskIO/DiskThreads/Makefile
src/DiskIO/IpcIo/Makefile
src/DiskIO/Mmapped/Makefile
src/engines/Makefile
src/error/Makefile
src/eui/Makefile
src/format/Makefile
Expand Down
11 changes: 0 additions & 11 deletions src/AsyncEngine.cc
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion src/EventLoop.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
/* DEBUG: section 01 Main Loop */

#include "squid.h"
#include "AsyncEngine.h"
#include "base/AsyncCallQueue.h"
#include "debug/Stream.h"
#include "EventLoop.h"
Expand Down
3 changes: 1 addition & 2 deletions src/EventLoop.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,13 @@
#ifndef SQUID_SRC_EVENTLOOP_H
#define SQUID_SRC_EVENTLOOP_H

#include "engines/AsyncEngine.h"
#include "time/forward.h"

#include <vector>

#define EVENT_LOOP_TIMEOUT 1000 /* 1s timeout */

class AsyncEngine;

/** An event loop. An event loop is the core inner loop of squid.
* The event loop can be run until exit, or once. After it finishes control
* returns to the caller. If desired it can be run again.
Expand Down
4 changes: 2 additions & 2 deletions src/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ SUBDIRS = \
time \
debug \
base \
engines \
anyp \
helper \
dns \
Expand Down Expand Up @@ -214,8 +215,6 @@ squid_SOURCES = \
$(WINSVC_SOURCE) \
AccessLogEntry.cc \
AccessLogEntry.h \
AsyncEngine.cc \
AsyncEngine.h \
AuthReg.h \
BodyPipe.cc \
BodyPipe.h \
Expand Down Expand Up @@ -536,6 +535,7 @@ squid_LDADD = \
$(ADAPTATION_LIBS) \
html/libhtml.la \
$(SNMP_LIBS) \
engines/libengines.la \
mem/libmem.la \
store/libstore.la \
time/libtime.la \
Expand Down
1 change: 0 additions & 1 deletion src/adaptation/ecap/ServiceRep.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "adaptation/ecap/Host.h"
#include "adaptation/ecap/ServiceRep.h"
#include "adaptation/ecap/XactionRep.h"
#include "AsyncEngine.h"
Copy link
Contributor

@rousskov rousskov Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not remove a header declaring a class that is used by the including code, even if some other included header happens to include that class declaration today. The same concern probably applies to some other modified files as well.

#include "base/TextException.h"
#include "debug/Stream.h"
#include "EventLoop.h"
Expand Down
26 changes: 12 additions & 14 deletions src/AsyncEngine.h → src/engines/AsyncEngine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,54 +6,52 @@
* Please see the COPYING and CONTRIBUTORS files for details.
*/

#ifndef SQUID_SRC_ASYNCENGINE_H
#define SQUID_SRC_ASYNCENGINE_H
#ifndef SQUID_SRC_ENGINES_ASYNCENGINE_H
#define SQUID_SRC_ENGINES_ASYNCENGINE_H

/* Abstract interface for async engines which an event loop can utilise.
/**
* Abstract interface for async engines which an event loop can utilise.
*
* Some implementations will be truly async, others like the event engine
* will be pseudo async.
*/

class AsyncEngine
{

public:
/* error codes returned from checkEvents. If the return value is not
* negative, then it is the requested delay until the next call. If it is
* negative, it is one of the following codes:
*/
enum CheckError {
/* this engine is completely idle: it has no pending events, and nothing
* registered with it that can create events
*/
/// This engine is completely idle: it has no pending events, and nothing
/// registered with it that can create events.
EVENT_IDLE = -1,
/* some error has occurred in this engine */
/// Some error has occurred in this engine.
EVENT_ERROR = -2
};

virtual ~AsyncEngine() {}

/* Check the engine for events. If there are events that have completed,
/**
* Check the engine for events. If there are events that have completed,
* the engine should at this point hand them off to their dispatcher.
* Engines that operate asynchronously - i.e. the DiskThreads engine -
* should hand events off to their dispatcher as they arrive rather than
* waiting for checkEvents to be called. Engines like poll and select should
* use this call as the time to perform their checks with the OS for new
* events.
*
* The return value is the status code of the event checking. If its a
* The return value is the status code of the event checking. If it is a
* non-negative value then it is used as hint for the minimum requested
* time before checkEvents is called again. I.e. the event engine knows
* how long it is until the next event will be scheduled - so it will
* return that time (in milliseconds).
*
* The timeout value is a requested timeout for this engine - the engine
* should not block for more than this period. (If it takes longer than the
* timeout to do actual checks that's fine though undesirable).
* timeout to do actual checks that is fine, though undesirable).
*/
virtual int checkEvents(int timeout) = 0;
};

#endif /* SQUID_SRC_ASYNCENGINE_H */

#endif /* SQUID_SRC_ENGINES_ASYNCENGINE_H */
17 changes: 17 additions & 0 deletions src/engines/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
## Copyright (C) 1996-2026 The Squid Software Foundation and contributors
##
## Squid software is distributed under GPLv2+ license and includes
## contributions from numerous individuals and organizations.
## Please see the COPYING and CONTRIBUTORS files for details.
##

include $(top_srcdir)/src/Common.am

noinst_LTLIBRARIES = libengines.la
Copy link
Contributor

@rousskov rousskov Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description: Squid main operational loop uses a set of Engines that perform various roles. This creates a library for the code underlying that logic.

Amos: This PR is an evolution of the previous attempts in PR #1548, with input from discussions in PR #1635. The consensus AFAICT is that we should have a library for this logic but not a specific namespace.

It is not clear to me what "underlying that logic" and "this logic" refers to here, so I cannot confirm that such a consensus exists. Creating a library consisting of one header file feels wrong, so I assume that this PR implies that some other code (containing some additional "logic") will be added to that library. That "other code" will help flesh out the actual library scope. We should decide what else should go into that library before finalizing and merging this PR. We do not need to agree on a comprehensive list, but we need to agree on more than just one header file. The comment quoted below may be a step towards making that decision:

Amos: the libengines.la supplies a shared hierarchy API class (this AsyncEngine) and perhaps the MainLoop/EventLoop code

Grouping AsyncEngine API and EventLoop code together makes sense, especially since that code is used by multiple executables today, but that group/library should not be called "engines" -- it provides no engines! A name like "MainLoop" or "EventLoop" may work (and would not preclude adding more engines to that library if we decide that those additions are a good idea).

Copy link
Contributor Author

@yadij yadij Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description: Squid main operational loop uses a set of Engines that perform various roles. This creates a library for the code underlying that logic.

Amos: This PR is an evolution of the previous attempts in PR #1548, with input from discussions in PR #1635. The consensus AFAICT is that we should have a library for this logic but not a specific namespace.

It is not clear to me what "underlying that logic" and "this logic" refers to here,

Updated the PR description.

FYI; abuse of a code change request to require changes to the PR description. Please use the review main comment for this type of thing.

Amos: the libengines.la supplies a shared hierarchy API class (this AsyncEngine) and perhaps the MainLoop/EventLoop code

Grouping AsyncEngine API and EventLoop code together makes sense, especially since that code is used by multiple executables today, but that group/library should not be called "engines" -- it provides no engines! A name like "MainLoop" or "EventLoop" may work (and would not preclude adding more engines to that library if we decide that those additions are a good idea).

As mentioned in my prior comment this PR is the basic infrastructure of the library. The other classes which inherit from AsyncEngine will also move in later in this refactor when their boundaries are cleaned up. Those are the actual "engines" it is named for providing (SignalEngine, TimeEngine, EventsEngine, ...) - same layout model as we use for HttpFooServer and FtpFooServer in libservers library, and equivalent in clients.

FTR; the model here is your preferred layout design. My own preferred layout is to separate each of these things by protocol. eg Http1Server and Http1Client would both be in src/http/ and TimeEngine would be in src/time/ and the trailing s on the library names would not be there. Anyway that ship has sailed years ago so I am just going with the model that has least resistance for this PR.


libengines_la_SOURCES = \
AsyncEngine.h

# XXX: autoconf only supplies CXXCOMPILE et al. properly when there is a .cc to build.
# XXX: temporary workaround for make check testHeaders auto-build.
EXCLUDE_FROM_HDR_TESTING += $(libengines_la_SOURCES)
2 changes: 1 addition & 1 deletion src/event.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
#ifndef SQUID_SRC_EVENT_H
#define SQUID_SRC_EVENT_H

#include "AsyncEngine.h"
#include "base/Packable.h"
#include "engines/AsyncEngine.h"
#include "mem/forward.h"

/* event scheduling facilities - run a callback after a given time period. */
Expand Down
1 change: 0 additions & 1 deletion src/tests/testEventLoop.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/

#include "squid.h"
#include "AsyncEngine.h"
#include "compat/cppunit.h"
#include "EventLoop.h"
#include "time/Engine.h"
Expand Down
Loading