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

Is it safe to have multiple ODBC environments #12

Closed
pacman82 opened this issue Feb 11, 2017 · 6 comments
Closed

Is it safe to have multiple ODBC environments #12

pacman82 opened this issue Feb 11, 2017 · 6 comments
Labels

Comments

@pacman82
Copy link
Contributor

May safe code allow having multiple ODBC environments? This link claims there should only be one Environment per Application (aka process) (http://old.vk.pp.ru/docs/sybase-any/interfaces/00000034.htm). However I did not find any such requirement in the official ODBC Documentation. Or did I just miss it?

@Koka
Copy link
Owner

Koka commented Feb 11, 2017

Looks like it's totally unsafe to allocate multiple environments. See https://odbcrouter.com/api/index.shtml?SQLAllocEnv for example.

On operating systems that support multiple threads, applications can use the same henv on different threads and drivers must therefore support safe, multithreaded access to this information. The application passes the henv value in all subsequent calls that require an henv.
There should never be more than one henv allocated at one time and the application should not call SQLAllocEnv when there is a current valid henv. If the application calls SQLAllocEnv with a pointer to a valid henv, the driver overwrites the henv without regard to its previous contents. (ODBCSDK/OverDRIVER returns the same handle.)

@pacman82
Copy link
Contributor Author

Seems to be true. The race conditions in the tests seem to indicate the same :-(. Despite having to prevent the user from instantiating the wrapper class more than once, this raises another troubling issue: Our library probably would not compose safely with other ODBC wrappers. I stated the issue with a call for help in this reddit thread. Maybe you want to weigh in.

@Koka
Copy link
Owner

Koka commented Feb 14, 2017

@pacman82 I think situation when someone uses two ODBC wrappers simultaneously in the same process is pretty exotic and probably the man who does this knows well what he's doing. It's good enough to prevent multiple instantiation of rust odbc environment wrapper and trust end user to not screw things up with another wrapper. And we need to ensure that our environment wrapper can be shared between threads comfortably - as it's allowed by odbc contract and user will have no ability to instantiate second environment.

@Koka
Copy link
Owner

Koka commented Feb 14, 2017

And may be we should allow unsafe construction of our wrapper from raw environment handle which can be allocated by another wrapper for example to give end user ability to use our wrapper with externally constructed environment?

@pacman82
Copy link
Contributor Author

Yesterday I discovered a bug I introduced in my fork (shame on me). Since fixing it I do no longer experience any data races while running tests. Reading the ODBC reference I did still not find any indication that allocating more than one Environment is UB. Yet we both did find secondary documentation claiming otherwise. In would suggest "in dubio pro reo": Our growing number of tests should eventually show if multiple Environments are a problem. Especially since they run in parallel. Until the tests show this to be the case I'm willing to assume multiple Environments are safe.

If you agree I would regard this issue as closed.

@Koka
Copy link
Owner

Koka commented Feb 15, 2017

Ok, let's leave to end user as original API do.

@Koka Koka closed this as completed Feb 15, 2017
@Koka Koka added the question label Mar 17, 2017
@pacman82 pacman82 mentioned this issue Mar 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants