Skip to content
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

Closed
wants to merge 1 commit into from

Conversation

bbdb68
Copy link

@bbdb68 bbdb68 commented Jan 12, 2017

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

…l behaviour

solution : explicitly call zsys_shutdown() before exit in selftest executable
@bluca
Copy link
Member

bluca commented Jan 12, 2017

Sorry but this can't work - it assumes the generated project is using CZMQ, but that might not be the case

@bbdb68
Copy link
Author

bbdb68 commented Jan 12, 2017

Ok I understand. What if we check with the template engine that czmq is amongst the dependencies ?
(altough I do not know how to do it)

@sappo
Copy link
Member

sappo commented Jan 12, 2017

No hard-coded dependencies please.

@bluca
Copy link
Member

bluca commented Jan 12, 2017

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

@bbdb68
Copy link
Author

bbdb68 commented Jan 12, 2017

@bluca it requires also changing all the tests for all projects using czmq.

@bluca
Copy link
Member

bluca commented Jan 12, 2017

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

@twhittock
Copy link
Contributor

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)

@bbdb68
Copy link
Author

bbdb68 commented Jan 13, 2017

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.

@bluca
Copy link
Member

bluca commented Jan 13, 2017

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.

@sappo
Copy link
Member

sappo commented Jan 13, 2017

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.

@bbdb68
Copy link
Author

bbdb68 commented Jan 13, 2017

in any case the solution will ne be at zproject level.

@bbdb68 bbdb68 closed this Jan 13, 2017
@bbdb68
Copy link
Author

bbdb68 commented Jan 15, 2017

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.

@bbdb68 bbdb68 reopened this Jan 15, 2017
@bluca
Copy link
Member

bluca commented Jan 19, 2017

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

@bluca bluca closed this Jan 19, 2017
@bbdb68 bbdb68 deleted the unit_test_exit branch January 21, 2017 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants