41

Let's say I have an object that provides some sort of functionality in an infinite loop.

Is is acceptable to just put the infinite loop in the constructor?

Example:

class Server {
    public:
    Server() {
        for(;;) {
            //...
        }
    }
};

Or is there an inherent initialization problem in C++ if the constructor never completes?

(The idea is that to run a server you just say Server server;, possibly in a thread somewhere...)

11
  • 14
    I don't think there's anything wrong with it but it's probably going to be confusing to users of your class. What's wrong with just putting the loop in a server::run function or similar?
    – Peter
    Commented Nov 7, 2020 at 11:31
  • 11
    Because most people (I assume) don't expect constructors to be blocking, let alone never return.
    – Peter
    Commented Nov 7, 2020 at 11:37
  • 15
    I am pretty sure your object is not valid until the constructor has run, so any access to your object from elsewhere would likely be undefined behavior.
    – Galik
    Commented Nov 7, 2020 at 11:40
  • 27
    Can you not just write a single function?
    – Galik
    Commented Nov 7, 2020 at 11:42
  • 35
    This is technically fine, but is IMHO a code smell. According to the single-responsibility principle, I wouldn't expect Server server; to create and run the server. Server server; server.run(); is way more readable and maintainable.
    – rustyx
    Commented Nov 7, 2020 at 11:56

6 Answers 6

76

It's not wrong per standard, it's just a bad design.

Constructors don't usually block. Their purpose is to take a raw chunk of memory, and transform it into a valid C++ object. Destructors do the opposite: they take valid C++ objects and turn them back into raw chunks of memory.

If your constructor blocks forever (emphasis on forever), it does something different than just turn a chunk of memory into an object. It's ok to block for a short time (a mutex is a perfect example of it), if this serves the construction of the object. In your case, it looks like your constructor is accepting and serving clients. This is not turning memory into objects.

I suggest you split the constructor into a "real" constructor that builds a server object and another start method that serves clients (by starting an event loop).

ps: In some cases you have to execute the functionality/logic of the object separately from the constructor, for example if your class inherit from std::enable_shared_from_this.

7
  • 4
    I'm with you in general, although it seems that modern constructors do not always just turn memory into objects. Your mutex example is one for example, or another example is the C++ thread constructor which launches the given function immediately. One could argue that creating a server is the role of the constructor... Commented Nov 8, 2020 at 10:53
  • 2
    A few things: 1) std::thread has some bad decisions behind it. this is why std::jthread was standardized. 2) I actuall would prefer the thread object to have a start method 3) It's true that sometimes we abuse our own principles. Software engineering principles are rules of thumb - usually they help us. seldomly, we break them because they hinder us from doing something simpler and more correct.
    – David Haim
    Commented Nov 8, 2020 at 12:12
  • 2
    Opening a file is something you might legitimately do in an RAII constructor; that can block for I/O for pathname lookup, potentially tens of milliseconds, or hundreds of ms on a heavily loaded system. Or whole seconds to spin up a magnetic drive that was idle. (Or NFS mount a server on mars...) Commented Nov 8, 2020 at 17:52
  • 2
    @CaptainCodeman is correct: RAII says a constructor should be more than just memory manipulation. Commented Nov 8, 2020 at 22:31
  • 7
    @PaulDraper: Emphasis on forever. Blocking to acquire a resource is very different from blocking forever. Commented Nov 9, 2020 at 8:47
18

It's allowed. But like any other non-trivial infinite loop, it must have observable side effects, otherwise you get undefined behavior.

Calling the networking functions counts as "observable side effects", so you're safe. This rule only bans loops that either do literally nothing, or just shuffle data around without interacting with the outside world.

6
16

Its legal, but its a good idea to avoid it.

The main issue is that you should avoid surprising users. Its unusual to have a constructor that never returns because it isn't logical. Why would you construct something you can never use? As such, while the pattern may work, it is unlikely to be an expected behavior.

A secondary issue is that it limits how your Server class can be used. The construction and destruction processes of C++ are fundamental to the language, so hijacking them can be tricky. For example, one might want to have a Server that is the member of a class, but now that overarching class' constructor will block... even if that isn't intuitive. It also makes it very difficult to put these objects into containers, as this can involve allocating many objects.

The closest I can think of to what you are doing is that of std::thread. Thread does not block forever, but it does have a constructor that does a surprisingly large amount of work. But if you look at std::thread, you realize that when it comes to multithreading, being surprised is the norm, so people have less trouble with such choices. (I am not personally aware of the reasons for starting the thread upon construction, but there's so many corner cases in multithreading that I would not be surprised if it resolves some of them)

4

A user might expect to set up your Server object in the main thread. Then call the server.endless_loop() function within a worker thread.

In an actual server, the process of acquiring a port requires escalated privileges which can then be dropped. Or perhaps you have an object that needs to load settings. Those sort of tasks could take place in the main thread before the long term looping takes place elsewhere.

Personally, I'd prefer your object had a "poll" function that was fast and non blocking. You could then have a loop function that called poll and sleep in an endless loop. You might even have an atomic variable that you can set to exit the loop from a different thread. Another feature would be to launch an internal thread within the Server object.

2

As others have pointed out, there's nothing "wrong" with this as far as C++ semantics are concerned, but it's poor design. The point of a constructor is to construct an object, so if that task never completes then it will be surprising to users.

Others have made suggestions regarding splitting the construction & run steps into constructor and method, which makes sense if you have other things you might want to do with the Server besides run it, or if you actually might want to construct it, do other stuff, and then run it.

But if you expect the caller will always just do Server server; server.run(), then maybe you don't even need a class -- it could just be a stand-alone function run_server(). If you don't have state to encapsulate and pass around in the first place, then you don't necessarily need objects. A stand-alone function can even be marked [[noreturn]] to make it clear both to the user and the compiler that the function never returns.

It's hard to say which makes more sense without knowing more about your use case. But in short: constructors construct objects -- if you're doing something else, don't use them for that.

2

In most cases, your code has no problem. Because of the following rule:

A class is considered a completely-defined object type ([basic.types]) (or complete type) at the closing } of the class-specifier. Within the class member-specification, the class is regarded as complete within function bodies, default arguments, noexcept-specifiers, and default member initializers (including such things in nested classes). Otherwise it is regarded as incomplete within its own class member-specification.

However, A restriction for your code is that you cannot use a glvalue that doesn't obtain from pointer this to access this object due to the behavior is unspecified. It's governed by this rule:

During the construction of an object, if the value of the object or any of its subobjects is accessed through a glvalue that is not obtained, directly or indirectly, from the constructor's this pointer, the value of the object or subobject thus obtained is unspecified.

Moreover, you cannot use the utility shared_ptr to manage such class objects. In general, place an infinitely loop into a constructor is not a good idea. Many restrictions will apply to the object when you use it.

Not the answer you're looking for? Browse other questions tagged or ask your own question.