6 ms·

Could you elaborate on why blocking in a thread's destructor is bad? You mention it is but don't really give any reason. Every time I use std::thread I find the

Could you elaborate on why blocking in a thread's destructor is bad? You mention it is but don't really give any reason. Every time I use std::thread I find the need to wait on it in the destructor. In fact any non-joining thread is a code smell to me. If you just detach (or abort...) a thread, then there's simply no way to ensure that main() will return after the thread has finished—and you always want to make sure all threads finish their tasks and clean up properly before the process terminates. This requires you to wait on threads during app shutdown, which generally corresponds with destruction, which means you need to join on destruction. I actually don't understand why you would want it another way by default (if at all).

gpderetta 6y ago
I just think that blocking is an important enough effect that that needs to be accounted for and it should be explicit in code. Deadlocking is the obvious reason, but also the possibly large pauses are problematic. I'm a big believer in crash-only software and I think that "clanup" at shutdown is code smell and a symptom of unreliable software [1]. Just call _Exit when the application is done. [1] Cleaning up to decrease the noise for the benefit of a leak detector is useful though, but by experience, I'm not yet convinced that the effort is worth it.
- OskarS 6y ago
  Just last week I was debugging an issue with a library that used SQLite internally, and I noticed that whenever I shut down, it would leave all these "something.db-shm" and "something.db-wal" files around in addition to the "something.db" file. My hunch was the library wasn't properly deinitializing SQLite, and indeed that was the case, if you call sqlite3_close() properly, these files are deleted (they're essentially journal files for SQLite). The library was essentially doing what you're suggesting, ending without doing the proper cleanup. Destruction, in general, is one of the most complex areas of software development in my opinion. You're right, blocking or throwing errors during destruction is terrible, but you really do have to genuinely clean things up when exiting, you can't leave messes behind like this. However, in addition to that: you can't ever count on destruction occuring, because it's entirely possible that your program crashes, the user kills the process, the computer runs out of battery (or someone janks the cable out) or some other random thing causes your process to exit prematurely. You really need a lot of nuance in this area, and I don't agree with "cleanup at shutdown is a code smell". But you're certainly right that you can go too far in the other direction as well: destruction has to be fast, non-blocking, error-free, and never required.
  - gpderetta 6y ago
    > you can't ever count on destruction occuring, because it's entirely possible that your program crashes, the user kills the process, the computer runs out of battery (or someone janks the cable out) or some other random thing causes your process to exit prematurely. Well yes, that's why you can't wait for shutdown time to cleanup your mess. In this case, either the files need to survive process exit or they can be unlinked as soon as they are created. Another option is to have very simple supervisor process whose only job is to cleanup. For a DB like sqlite it does makes sense to have some amount of shutdown cleanup to close transactions and mark the log as stale and avoid any recovery on startup, but it should be considered an optimization and not be a requirement. To make this short, I do in fact agree with your nuanced view. Pragmatism always trumps dogmatism in software engineering.
    - dataflow 6y ago
      > In this case, either the files need to survive process exit or they can be unlinked as soon as they are created. The latter literally isn't possible on Windows. And I'd argue it's a good thing because you shouldn't do that in this case, though either way you don't have that option anyway. The notion of having a supervisory process just to clean up your mess that you had every chance to clean up is just poor design. Supervisory processes are for when you can't do that and there's critical work left over, not for cleaning up random clutter you just didn't bother to try to handle at the correct time. Imagine if every program that used SQLite spawned an extra supervisory process? Now imagine if this was true for every program that needed temporary files as long as it was running? That's obviously just terrible. Your code needs to clean up after itself as much as it can, whether via RAII or otherwise, but you can't just throw out the notion of cleaning things up on shutdown (or any other point) altogether!
      - gpderetta 6y ago
        you do not have a chance to cleanup if you are application is killed anyway. So either don't make a mess, make sure that you can clean up no matter what or live with it.
simiones 6y ago
I think there are two schools of thought on this issue: 1. Blocking/unblocking is a major side effect, so it should be handled automatically so you don't forget about it 2. Blocking/unblocking is a major side effect, so it should be very prominent in the code so you don't miss it when reading, and it doesn't accidentally happen when you don't expect it The C++ designers usually favor the first school, leading to designs like `std::lock_guard`. Many others, myself included, favor the second school, and would complain that the code marker that a method will block forever waiting for a thread to finish shouldn't be "}". I would also say that in general blocking forever should not be the default - you should have to explicitly ask for it, not have it as a default. This could be achieved with a RAII design by passing the timeout parameter to the constructor, but that obscures the meaning of the } even more, especially if the thread ownership ever changes hands (a function receiving a std::unique_ptr<std::jthread> would wait an unknown amount of time on a }, for example).
- gpderetta 6y ago
  So, I guess I fall in the middle. I have no issues with lock_guard. Also, it will unlock on destruction, which is not much of a problem. And having this sort of side effect at end of scope is consistent with languages having synchronized or atomic blocks. I would also not have much of a problem with a join_guard that joins one or more thread on destruction. I am a fan of RAII and both lock_guard and join_guard have a single job and if you use them it is pretty obvious what you want, you can opt in on them and you can delegate the job by passing them around. My issue is with more complex objects blocking on destruction when it is not their primary purpose.
- dataflow 6y ago
  The thing is, you talk about how blocking implicitly is bad, but I didn't say "it's good to block in the destructor" as a blanket rule either. I'm talking about this very particular case of blocking deserves to be in the destructor, for what I thought were fairly decent reasons that I spelled out. You haven't actually addressed my points about threads specifically and are just talking about blocking in a destructor in general, but the discussion/disagreement isn't about other cases. There are definitely times when I think it'd be wrong to block implicitly, but this just isn't one of them.

gpderetta 6y ago

I just think that blocking is an important enough effect that that needs to be accounted for and it should be explicit in code. Deadlocking is the obvious reason, but also the possibly large pauses are problematic. I'm a big believer in crash-only software and I think that "clanup" at shutdown is code smell and a symptom of unreliable software [1]. Just call _Exit when the application is done. [1] Cleaning up to decrease the noise for the benefit of a leak detector is useful though, but by experience, I'm not yet convinced that the effort is worth it.

OskarS 6y ago
Just last week I was debugging an issue with a library that used SQLite internally, and I noticed that whenever I shut down, it would leave all these "something.db-shm" and "something.db-wal" files around in addition to the "something.db" file. My hunch was the library wasn't properly deinitializing SQLite, and indeed that was the case, if you call sqlite3_close() properly, these files are deleted (they're essentially journal files for SQLite). The library was essentially doing what you're suggesting, ending without doing the proper cleanup. Destruction, in general, is one of the most complex areas of software development in my opinion. You're right, blocking or throwing errors during destruction is terrible, but you really do have to genuinely clean things up when exiting, you can't leave messes behind like this. However, in addition to that: you can't ever count on destruction occuring, because it's entirely possible that your program crashes, the user kills the process, the computer runs out of battery (or someone janks the cable out) or some other random thing causes your process to exit prematurely. You really need a lot of nuance in this area, and I don't agree with "cleanup at shutdown is a code smell". But you're certainly right that you can go too far in the other direction as well: destruction has to be fast, non-blocking, error-free, and never required.
- gpderetta 6y ago
  > you can't ever count on destruction occuring, because it's entirely possible that your program crashes, the user kills the process, the computer runs out of battery (or someone janks the cable out) or some other random thing causes your process to exit prematurely. Well yes, that's why you can't wait for shutdown time to cleanup your mess. In this case, either the files need to survive process exit or they can be unlinked as soon as they are created. Another option is to have very simple supervisor process whose only job is to cleanup. For a DB like sqlite it does makes sense to have some amount of shutdown cleanup to close transactions and mark the log as stale and avoid any recovery on startup, but it should be considered an optimization and not be a requirement. To make this short, I do in fact agree with your nuanced view. Pragmatism always trumps dogmatism in software engineering.
  - dataflow 6y ago
    > In this case, either the files need to survive process exit or they can be unlinked as soon as they are created. The latter literally isn't possible on Windows. And I'd argue it's a good thing because you shouldn't do that in this case, though either way you don't have that option anyway. The notion of having a supervisory process just to clean up your mess that you had every chance to clean up is just poor design. Supervisory processes are for when you can't do that and there's critical work left over, not for cleaning up random clutter you just didn't bother to try to handle at the correct time. Imagine if every program that used SQLite spawned an extra supervisory process? Now imagine if this was true for every program that needed temporary files as long as it was running? That's obviously just terrible. Your code needs to clean up after itself as much as it can, whether via RAII or otherwise, but you can't just throw out the notion of cleaning things up on shutdown (or any other point) altogether!
    - gpderetta 6y ago
      you do not have a chance to cleanup if you are application is killed anyway. So either don't make a mess, make sure that you can clean up no matter what or live with it.

simiones 6y ago

I think there are two schools of thought on this issue: 1. Blocking/unblocking is a major side effect, so it should be handled automatically so you don't forget about it 2. Blocking/unblocking is a major side effect, so it should be very prominent in the code so you don't miss it when reading, and it doesn't accidentally happen when you don't expect it The C++ designers usually favor the first school, leading to designs like `std::lock_guard`. Many others, myself included, favor the second school, and would complain that the code marker that a method will block forever waiting for a thread to finish shouldn't be "}". I would also say that in general blocking forever should not be the default - you should have to explicitly ask for it, not have it as a default. This could be achieved with a RAII design by passing the timeout parameter to the constructor, but that obscures the meaning of the } even more, especially if the thread ownership ever changes hands (a function receiving a std::unique_ptr<std::jthread> would wait an unknown amount of time on a }, for example).

gpderetta 6y ago
So, I guess I fall in the middle. I have no issues with lock_guard. Also, it will unlock on destruction, which is not much of a problem. And having this sort of side effect at end of scope is consistent with languages having synchronized or atomic blocks. I would also not have much of a problem with a join_guard that joins one or more thread on destruction. I am a fan of RAII and both lock_guard and join_guard have a single job and if you use them it is pretty obvious what you want, you can opt in on them and you can delegate the job by passing them around. My issue is with more complex objects blocking on destruction when it is not their primary purpose.
dataflow 6y ago
The thing is, you talk about how blocking implicitly is bad, but I didn't say "it's good to block in the destructor" as a blanket rule either. I'm talking about this very particular case of blocking deserves to be in the destructor, for what I thought were fairly decent reasons that I spelled out. You haven't actually addressed my points about threads specifically and are just talking about blocking in a destructor in general, but the discussion/disagreement isn't about other cases. There are definitely times when I think it'd be wrong to block implicitly, but this just isn't one of them.