asio::demuxer at global scope dead-locks?
I tried modified the timer1.cpp a little: #include <iostream> #include <asio.hpp> asio::demuxer d; int main() { asio::deadline_timer t(d, boost::posix_time::seconds(5)); t.wait(); std::cout << "Hello, world!\n"; return 0; } by moving the demuxer object to global scope. I have to be honest, I am not through with the documentation yet, but also I did not find a bold mark which prohibits such use. At least the program seems to never end, it seems to hang somewhere in the library clean up code? I can see it waiting for a thread to stop: // Wait for the thread to exit. void join() { ::WaitForSingleObject(thread_, INFINITE); } But this never happens. Is it true indeed, that demuxer s must not be at global scope? Regards, Roland
Hi Roland, --- Roland Schwarz <roland.schwarz@chello.at> wrote:
Is it true indeed, that demuxer s must not be at global scope?
It's only that nobody has thought to try it before :) The problem arises because WSACleanup is called while the background select thread is still running. Applying the following change to include/asio/basic_demuxer.hpp should fix the problem: @@ -229,6 +229,10 @@ } private: +#if defined(_WIN32) + detail::winsock_init<> winsock_init_; +#endif + /// The service registry. detail::service_registry<basic_demuxer<Demuxer_Service> > service_registry_; Cheers, Chris
Christopher Kohlhoff schrieb:
The problem arises because WSACleanup is called while the background select thread is still running. Applying the following change to include/asio/basic_demuxer.hpp should fix the problem:
@@ -229,6 +229,10 @@ }
private: +#if defined(_WIN32) + detail::winsock_init<> winsock_init_; +#endif + /// The service registry. detail::service_registry<basic_demuxer<Demuxer_Service> > service_registry_;
Yes it apparently fixes the problem, but as I can currently see, it defeats the original purpose of the single call to WSAStartup. It seems not to be necessary in this case. What the code is trying to achive is kind of a call_once, as I can currently see. But it delegates instead to the operating system by calling WSAStartup multiple times. While it seems to be legal to call it multiple times there are some questions: 1) Is the call to WSAStartup thread safe? (Coudl not find something definitive, but it seems so.) 2) There was an issue with memory leaking in the WSAStartup code, so some (older) windows versions might be affected. http://support.microsoft.com/?scid=kb%3Ben-us%3B237572&x=12&y=10 3) The call maight fail: e.g.: WSAEINPROGRESS, WSAEPROCLIM, ... how will you be able to pair the matching number of WSACleanup calls? Why not simply use boost threads once function instead? I also discovered another issue: The number of demuxers is implicitely limited by available TSS slots. This also is an issue that long ago has been succesfully solved in the threading library. A final question is poping up: Why not using the boost threading? Or is this planned for future? Regards, Roland
Hi Roland, --- Roland Schwarz <roland.schwarz@chello.at> wrote:
Yes it apparently fixes the problem, but as I can currently see, it defeats the original purpose of the single call to WSAStartup. It seems not to be necessary in this case. What the code is trying to achive is kind of a call_once, as I can currently see.
Yes, although the problem in this case is not so much calling WSAStartup once, but ensuring that WSACleanup is called after all demuxer objects have been destroyed (including global ones).
Why not simply use boost threads once function instead?
Does it address the issue of order of cleanup with respect to globals?
I also discovered another issue: The number of demuxers is implicitely limited by available TSS slots. This also is an issue that long ago has been succesfully solved in the threading library.
Yes, although the demuxer's use of tss is to keep track of whether the current thread is "in" the demuxer. The problem I had with the boost tss when I first investigated it a couple of years ago was that it seemed to involve storing an object in tss, when all I really wanted was a thread-specific boolean flag (and I had no need for cleanup). However I can think of an implementation that would only use one tss slot for all demuxers, at a small runtime cost in unusual use cases. I will investigate doing that for 0.3.4.
A final question is poping up: Why not using the boost threading? Or is this planned for future?
It doesn't use boost threads internally so that it can keep the library header-file only, i.e. to avoid adding a dependency on any non-system DLLs or libraries. But if a future boost threads release was header-file-only for the most commonly used bits (threads, mutex, condition)... ;) Cheers, Chris
Christopher Kohlhoff wrote:
--- Roland Schwarz <roland.schwarz@chello.at> wrote:
Why not simply use boost threads once function instead?
Does it address the issue of order of cleanup with respect to globals?
This is indeed one of the hardest problems, specially when dealing with global objects. E.g. there is a problem when the main thread is ending, but some other thread are living even longer. I vaguely remeber some recommendation not using these globals in MT at all. But I am not too sure. Also this sounds much to prohibitive. Me for my part always ended with some kind of reference counting. This btw. is what WSAStartup/Cleanup seemingly is doing inside. But I think you could simply move the ref-counting into your code to be independant of WSAStartups peculiarities. E.g. you could simply wrap up things into a shared_ptr.
I also discovered another issue: The number of demuxers is implicitely limited by available TSS slots. This also is an issue that long ago has been succesfully solved in the threading library.
Yes, although the demuxer's use of tss is to keep track of whether the current thread is "in" the demuxer. The problem I had with the boost tss when I first investigated it a couple of years ago was that it seemed to involve storing an object in tss, when all I really wanted was a thread-specific boolean flag (and I had no need for cleanup).
Not sure if I understand this correctly. You are using ::TLSFree don't you? Besides how about boost::thread_specific_ptr<bool> pbool; pbool.reset(new bool); ? The thread_specific_ptr is doing almost what your class win_tss_bool is needing to do anyways.
A final question is poping up: Why not using the boost threading? Or is this planned for future?
It doesn't use boost threads internally so that it can keep the library header-file only, i.e. to avoid adding a dependency on any non-system DLLs or libraries.
I know this might seem a little ignorant, but could you please give me your rationale for "header only" ?
But if a future boost threads release was header-file-only for the most commonly used bits (threads, mutex, condition)... ;)
There is currently a rewrite going on. I will try to drop this into our bag, but I am not currently convinced that header only really is a win, but I would like you proving me wrong. Good arguments could give raise to a header only implementation of basic locking primitives too. Regards, Roland (Speedsnail)
Hi Roland, --- Roland Schwarz <roland.schwarz@chello.at> wrote:
Me for my part always ended with some kind of reference counting. This btw. is what WSAStartup/Cleanup seemingly is doing inside. But I think you could simply move the ref-counting into your code to be independant of WSAStartups peculiarities. E.g. you could simply wrap up things into a shared_ptr.
I have been thinking about this today, and that is also what I have decided to do.
Not sure if I understand this correctly. You are using ::TLSFree don't you?
Yes, but...
Besides how about boost::thread_specific_ptr<bool> pbool; pbool.reset(new bool); ?
the boost tss stuff seems to include a lot of code to ensure that the newed bool is deleted at the appropriate time. This is unnecessary when I just want to store a boolean value - i.e. I just store a null-pointer to mean false, a non-null pointer to mean true. In the change I'm planning to make it use one tss slot for all demuxers, I still do not need any cleanup, since i will simply be storing pointers to stack-based objects in tss.
I know this might seem a little ignorant, but could you please give me your rationale for "header only" ?
It's very simple, and mostly non-technical: I see having a compiled library as an entry barrier to people using my library. I want to make it as easy as possible for people to get started and use. A compiled library means something has to be built before developers can get started developing. Sometimes this means waiting for a very long build to complete when you only want to use a small part of the library. Sometimes the build step is a potential source of errors for the user if they don't follow the instructions correctly, and therefore results in support emails that have to be answered. You also have to support building in all the common configurations (I realise Boost.Build does this for you). And this build step is required every time you release a new version. With a header file library, the steps to start using it are very simple: - Is your compiler/platform supported? if so.. - Download the library and unpack it - Add it to your include path - Start coding :) (Obviously it helps if the library is predominantly template-based, and if you want to be able to inline most OS calls for performance reasons.) Cheers, Chris
Christopher Kohlhoff schrieb:
the boost tss stuff seems to include a lot of code to ensure that the newed bool is deleted at the appropriate time. This is unnecessary when I just want to store a boolean value - i.e. I just store a null-pointer to mean false, a non-null pointer to mean true.
What I mean is this: You also need to take care to call TLSFree at the correct time. So this makes no difference. Could it be that the "real" reason indeed also is to the header only argument?
It's very simple, and mostly non-technical: I see having a compiled library as an entry barrier to people using my library.
Altough this currently is not a boost philosophy: Do you think prebuilt binaries could change this? Besides e.eg. on debian this a non-issue, since you always simply can upgrade to the most recent boost lib. (At least this is what I have seen.)
I want to make it as easy as possible for people to get started and use.
But you force the user to longer compile times and the hassle of precompiled header manipulation for larger projects, don't you? Regards, Roland
Hi Roland, --- Roland Schwarz <roland.schwarz@chello.at> wrote:
What I mean is this: You also need to take care to call TLSFree at the correct time. So this makes no difference.
Not quite. The TLSFree only needs to be done once for the tss object. But the newed bool must be freed for each thread, in each thread, when the thread exits. That is what I wanted to avoid. I just remembered that I also wanted to allow the threads that call demuxer::run to be created using whatever API the developer prefers, be it boost::thread, ACE, pthread_create or _beginthreadex. AFAIK to use boost's tss classes portably you also have to use boost::thread throughout your app to ensure the cleanup handlers get called. Is that right? It makes me think it would be good to have a non-cleaning-up tss abstraction in boost. If there was one, asio could use it.
Could it be that the "real" reason indeed also is to the header only argument?
That may be one of the reasons, but it's definitely not the only one.
Altough this currently is not a boost philosophy: Do you think prebuilt binaries could change this?
Perhaps, but it would be a lot of work to generate the builds for all the different supported combinations. It would also have to use autolinking wherever possible.
But you force the user to longer compile times and the hassle of precompiled header manipulation for larger projects, don't you?
Yes, perhaps, but my idea is that by the time the developer wants to use the library in larger projects, they are willing to commit time to use it more efficiently (such as using precompiled headers). It's the new user who's just evaluating it that needs the process to be as painless as possible. Cheers, Chris
Christopher Kohlhoff schrieb:
--- Roland Schwarz <roland.schwarz@chello.at> wrote:
What I mean is this: You also need to take care to call TLSFree at the correct time. So this makes no difference.
Not quite. The TLSFree only needs to be done once for the tss object. But the newed bool must be freed for each thread, in each thread, when the thread exits. That is what I wanted to avoid.
Ok, I see. You could do away with the auto cleanup by specifying a custom do nothing handler, but that certainly is not what you would want to do, since you would need ugly casts from pointer to bool. I also remember that I was not too comfortable with having TSS only as pointers. But you need this, otherwise a complete constructor call would be incured at every thread startup. Perhaps it is a good idea to add support for POD's into TSS...?
I just remembered that I also wanted to allow the threads that call demuxer::run to be created using whatever API the developer prefers, be it boost::thread, ACE, pthread_create or _beginthreadex. AFAIK to use boost's tss classes portably you also have to use boost::thread throughout your app to ensure the cleanup handlers get called. Is that right?
I always was a little unfomfortable with this too. As I see it there is not really a need for this coupling. Altough thread could benefit from TSS usage, but not necessarily the other way round. I would rather like to see the thread_specific_ptr belonging to the smart_ptr's. Besides this the coupling does exist only in the boost headers and does not force you to use a certain threading API at all. In the opposite one of the strenghts of boost::threads TSS implementation is its independance of the threading API despite the fact that it is able to do constructor/destructor calls.
It makes me think it would be good to have a non-cleaning-up tss abstraction in boost. If there was one, asio could use it.
Perhaps this should be attempted for PODS. <snip>
Perhaps, but it would be a lot of work to generate the builds for all the different supported combinations. It would also have to use autolinking wherever possible.
This already has been solved. The only thing missing is an installer for windows. (Not sure about gcc. Does anyone know?) <snip>
Yes, perhaps, but my idea is that by the time the developer wants to use the library in larger projects, they are willing to commit time to use it more efficiently (such as using precompiled headers). It's the new user who's just evaluating it that needs the process to be as painless as possible.
Does the precompiled header thing also work for gcc on windows? Besides this I always found it much more painful to get precompiled headers working satisfactory. But this only my personal opinion. Regards, Roland
participants (2)
-
Christopher Kohlhoff -
Roland Schwarz