-
Notifications
You must be signed in to change notification settings - Fork 104
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.
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
problem : on windows, unit tests do not exit gracefully because of dl… #836
Conversation
…l behaviour solution : explicitly call zsys_shutdown() before exit in selftest executable
Sorry but this can't work - it assumes the generated project is using CZMQ, but that might not be the case |
Ok I understand. What if we check with the template engine that czmq is amongst the dependencies ? |
No hard-coded dependencies please. |
Yes it could get hairy. What if, instead, we change each self test method in czmq to call shutdown on windows? (The tests are manually written) This way we don't pollute zproject |
@bluca it requires also changing all the tests for all projects using czmq. |
If calling zsys_shutdown is a requirement on Windows, then all projects using CZMQ need to call it at exit I'm afraid, like any other library clean up |
I removed the call in CZMQ selftest as a part of zeromq/czmq@5ea2e81 - I also added it in the first place... :) I'm not sure zproject is a totally generic project definition tool, but it does already have knowledge of some libraries so really where should that knowledge stop? There are plenty of other libraries out there which require setup/teardown code, so is it actually just another step towards zproject dealing with more boilerplate for you? Regarding adding startup/shutdown per test, I'm not sure CZMQ actually supports that happening multiple times in the same process (at least on Windows) |
When I read the RFC 21, I understand that for a zproject based project, dependency on czmq is required... may be it is no more the case ? On the other hand, reading the posts provided by @twhittock , I understand that the atexit call is non portable, which makes the czmq api currently non portable. IMHO the clean solution to this would be reduce dependency on non portable system calls and make the API the same on all systems, which of course makes explicit shutdown mandatory on all platforms. I also understand this can be a major change not easy to carry. Having to handle platform dependency on every test case sounds as an API issue do me. |
I think it used to be required, but I don't think it is anymore. Regarding atexit, I don't think we should cripple all platforms because of a Windows bug. The atexit function is part of POSIX.1-2001 after all, it's not Linux specific. When we have a platform specific bug we work around it usually, we don't take the feature away. |
Regarding the code style it is independent of czmq but there are a lot of rules on how to communicate with threads for example that require czmq. |
in any case the solution will ne be at zproject level. |
I repoen for another "what if". What about implementing a ref counting mecanism in the classes that directly use the network (zsock and zbeacon ?) so that zsys_shutdown is called on the last destroy of a czmq object ? This would not change the API and make atexit useless. |
I've rerun the tests a couple of times and they do not seem to have problems with calling shutdown at the end of each selftest: https://ci.appveyor.com/project/bluca/czmq/build/build-325 CMake does one run for each test individually, so it shouldn't be an issue. That refcounting wouldn't work as a user might be destroying an object but might not be done with the running, and create another one. Also from the point of view of the implementation, we really don't want to entangle all the classes together like that. The zsys_shutdown documentation already mentioned problems with Windows DLL. The right solution here is to make it unambiguous for that case. I've implemented this here: zeromq/czmq#1608 |
problem : on windows, unit tests do not exit gracefully because of dll behaviour (atexit not called)
solution : explicitly call zsys_shutdown() before exit in selftest executables