Yes, you're right, sorry if I missed that.

We don't actually. I initially couldn't get this working without it for some reason but now I see it works just alright.

Honestly, it's hard to say whether it throws or not, there's no information in the docs, I tried to dig into the implementation but there are a lot of system calls, etc. there and I'm not sure if it's worth it to reverse engineer this to the bottom of it, especially that...

I think we can just delete this line. This is a singleton, which means it has just one single, static instance. This means that this instance is going to be released when the whole program goes down. And the server basically loops forever and the only case that it terminates is when something goes really bad (that shouldn't occur). I don't see a scenario where this gets released gracefully, therefore join doesn't seem to be really necessary here.

I was thinking about this for a bit and I don't think this is a good idea, sorry.

The problem with calling one method from the other is that passing arguments would be problematic as we should receive an unknown number of unknown arguments that should be passed to the callback.

I just don't think it makes sense to write functionalities that are not going to be used - and that would be a function schedule(Task) - we're not going to use it anywhere. And if anyone ever needs something like this, it can easily be introduced like this:

void schedule(Task task) {
  boost::asio::post(this->pool, [task]() {
    try {
      task();
    } catch (std::exception &e) {
      LOG(ERROR) << "error occured in a scheduled task: " << e.what();
    }
  });
}

I'm ok though with renaming the current function to something like scheduleWithCallback.

This is a link to the most recent doc https://www.boost.org/doc/libs/1_80_0/doc/html/boost_asio/reference/thread_pool/_thread_pool.html

It's a part of the singleton pattern. By doing it, we forbid creating additional instances and have just one.

We might, but I think that's okay. Alternatives we have:

• pass them as references - this is most dangerous and would probably crash right away. If the originals get out of scope (deallocated), we'd end up with an invalid access crash.
• move them - this is tricky in c++ and would probably require wrapping them into unique_ptrs.

Having these options, I still lean towards the current solution. I don't think copying them is that expensive.

Oh crap, forgot to rename it, sorry about this.

I disagree, sorry.

we still should destruct it gracefully so that all the work can be finished, regardless the reason

I already stated that this is impossible. The destructor of this singleton's never going to be called. The only scenario in which it stops working is when the service crashes. And in this case, all the threads are going to be stopped forcibly anyway,

What's wrong with having two or more pools?

I think this is unacceptable. I think we should never have multiple pools (https://stackoverflow.com/a/35328044/15854120). Maybe if there were a couple of different "components" doing different jobs, but having multiple pools for one purpose? I think it looks more like an anti-pattern. We can always extend the thread limit, but overall, I'd go for more control of what's going on.

Then we could have a singleton that holds an instance of this class

This doesn't make sense at all to me. If we have class A that is a singleton, why would we ever come up with a solution where A is not a singleton and we create an additional class B that is a singleton and holds an instance of A? It's equivalent, we just have an additional layer of abstraction that doesn't do anything really. did I miss something here?

I think the current solution will work just alright.

I just don't understand why we would want to have many instances of thread pools flying around. This introduces a lot of complexity and becomes error-prone (we really need to be in control of their state etc.)

The SO post you provided doesn't really get anything new to the table I think. It's all true, the point is that I perceive the grpc server as an activity/module/component that does exactly one job. Let's say we had another component that performs file i/o, then creating another thread pool for this would totally make sense. But here, there are the same tasks basically but for different clients.

A good question would be: how would you specify what the exact number of thread pools should be? On what that would be based?

Layers of abstraction sometimes doesn't introduce any new logic, but instead improve readability, maintainability, and overall design of the system.

I understand, but in this case, I just don't see a profit at all. It just brings more complexity and some redundancy. What exactly is the advantage of this approach?

Let's keep the current design for now. We will improve this when necessary.

Let's do it, the discussion itself is interesting though.

It's all true, the point is that I perceive the grpc server as an activity/module/component that does exactly one job. Let's say we had another component that performs file i/o, then creating another thread pool for this would totally make sense. But here, there are the same tasks basically but for different clients.

I think we agree but weren't communicating effectively. I wasn't proposing that we should have a couple of pools in this module. I was advising that the class should be written in a reusable way.

I understand, but in this case, I just don't see a profit at all. It just brings more complexity and some redundancy. What exactly is the advantage of this approach?

The class would be more reusable, so using it in a different context would be cheap and would not cause any risk of breaking existing usages.

I now fully understand, and that makes sense (this class resides in fact in the lib directory). I think this can be done on demand as you said earlier.