[Libdbi-drivers-devel] freetds driver, single instance?
Hi, As it seems we're a bit unlucky at these times. Now I've ran into a problem in the FreeTDS driver, we're getting heap corruptions inside freetds, and as it seems the root cause is the DBD freetds driver. More exactly, dbd_freetds seems to have a single state instance, instead of allocating a new state for each connection: typedef struct freedts_type { CS_CONTEXT *ctx; CS_CONNECTION *conn; CS_COMMAND *cmd; } FREETDSCON; static FREETDSCON freetds; ^ int dbd_connect(dbi_conn_t * conn) { FREETDSCON *tdscon = freetds; This means that if an application uses two DBI contexts at the same time, those will clash and cause a segfault. Does anyone know why this limitation was added to the freetds driver? I'm experimenting with the following patch, but still testing it. I'd appreciate if you could review it for mistakes I possibly made. BTW: what is the policy about commented out code in libdbi/libdbi-drivers source? I personally feel that commented out code is not good practice, especially if the code in question is commented out without a note stating the exact reason why it was done. For me it makes the code harder to read. -- Bazsi commit 6ec70cf9b3d780a5cceb584ad28ded7a6ec31196 Author: Balazs Scheidler [EMAIL PROTECTED] Date: Tue Aug 5 10:59:19 2008 +0200 allow the use of multiple parallel freetds connections diff --git a/drivers/freetds/dbd_freetds.c b/drivers/freetds/dbd_freetds.c index 449c313..b869e8b 100644 --- a/drivers/freetds/dbd_freetds.c +++ b/drivers/freetds/dbd_freetds.c @@ -56,7 +56,6 @@ typedef struct freedts_type { CS_CONNECTION *conn; CS_COMMAND *cmd; } FREETDSCON; -static FREETDSCON freetds; static const dbi_info_t driver_info = { freetds, @@ -150,12 +149,13 @@ int dbd_initialize(dbi_driver_t * driver) int dbd_connect(dbi_conn_t * conn) { - -FREETDSCON *tdscon = freetds; +FREETDSCON *tdscon; CS_RETCODE ret; char *str; unsigned int num; + +tdscon = malloc(sizeof(FREETDSCON)); /* * Allocate memory for structs */ @@ -174,7 +174,7 @@ int dbd_connect(dbi_conn_t * conn) /* Deallocate ctx struct */ cs_ctx_drop(tdscon-ctx); } - +free(tdscon); return -1; success_allocate: conn-connection = tdscon; @@ -273,7 +273,7 @@ int dbd_disconnect(dbi_conn_t * conn) ct_con_drop(tdscon-conn); ct_exit(tdscon-ctx, CS_UNUSED); cs_ctx_drop(tdscon-ctx); - +free(tdscon); return 0; } - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/___ Libdbi-drivers-devel mailing list Libdbi-drivers-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libdbi-drivers-devel
Re: [Libdbi-drivers-devel] freetds driver, single instance?
Quoting Balazs Scheidler [EMAIL PROTECTED]: Hi, As it seems we're a bit unlucky at these times. Now I've ran into a problem in the FreeTDS driver, we're getting heap corruptions inside freetds, and as it seems the root cause is the DBD freetds driver. More exactly, dbd_freetds seems to have a single state instance, instead of allocating a new state for each connection: typedef struct freedts_type { CS_CONTEXT *ctx; CS_CONNECTION *conn; CS_COMMAND *cmd; } FREETDSCON; static FREETDSCON freetds; ^ This is certainly against the idea of being able to create as many connections as you need. Afaict no other driver suffers from such a limitation. Does anyone know why this limitation was added to the freetds driver? I'm afraid no. Vadym (who developed the driver) is no longer active as he does not seem to work with MS SQL Server anymore, but I'll forward your mail to him and see whether he can explain. I'm experimenting with the following patch, but still testing it. I'd appreciate if you could review it for mistakes I possibly made. Your patch looks good to me although I can't test-drive it. BTW: what is the policy about commented out code in libdbi/libdbi-drivers source? I personally feel that commented out code is not good practice, especially if the code in question is commented out without a note stating the exact reason why it was done. For me it makes the code harder to read. There is no such thing as a policy right now. If anyone feels like we should have a policy, I'll be happy to help finding or writing one. There are quite a few things that we could improve, from aesthetic (C vs. C++ style comments) to good practice (variable names, comments describing the usage and parameter list of functions). I use commented-out code mainly for printf() debugging which I personally feel is reasonable. I agree that blocks of commented-out code should be avoided or at least explained and signed. regards, Markus -- Markus Hoenicka [EMAIL PROTECTED] (Spam-protected email: replace the quadrupeds with mhoenicka) http://www.mhoenicka.de - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/ ___ Libdbi-drivers-devel mailing list Libdbi-drivers-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libdbi-drivers-devel
Re: [Libdbi-drivers-devel] freetds driver, single instance?
On Tue, 2008-08-05 at 12:35 +0200, Markus Hoenicka wrote: Quoting Balazs Scheidler [EMAIL PROTECTED]: Hi, As it seems we're a bit unlucky at these times. Now I've ran into a problem in the FreeTDS driver, we're getting heap corruptions inside freetds, and as it seems the root cause is the DBD freetds driver. More exactly, dbd_freetds seems to have a single state instance, instead of allocating a new state for each connection: typedef struct freedts_type { CS_CONTEXT *ctx; CS_CONNECTION *conn; CS_COMMAND *cmd; } FREETDSCON; static FREETDSCON freetds; ^ This is certainly against the idea of being able to create as many connections as you need. Afaict no other driver suffers from such a limitation. Does anyone know why this limitation was added to the freetds driver? I'm afraid no. Vadym (who developed the driver) is no longer active as he does not seem to work with MS SQL Server anymore, but I'll forward your mail to him and see whether he can explain. I'm experimenting with the following patch, but still testing it. I'd appreciate if you could review it for mistakes I possibly made. Your patch looks good to me although I can't test-drive it. It works with light testing, e.g. now it does not crash where it used to. I did not check for leaks or similar stuff, however I think I only needed to free the allocated memory in dbd_disconnect() and in case of an error. I've added both of these spots, so it should be ok in this regard. We are installing the patch in production, but now I think it is safe to commit it. BTW: what is the policy about commented out code in libdbi/libdbi-drivers source? I personally feel that commented out code is not good practice, especially if the code in question is commented out without a note stating the exact reason why it was done. For me it makes the code harder to read. There is no such thing as a policy right now. If anyone feels like we should have a policy, I'll be happy to help finding or writing one. There are quite a few things that we could improve, from aesthetic (C vs. C++ style comments) to good practice (variable names, comments describing the usage and parameter list of functions). I use commented-out code mainly for printf() debugging which I personally feel is reasonable. I agree that blocks of commented-out code should be avoided or at least explained and signed. Just let me know if you accept patches that remove these kind of code blocks. (for instance the oracle compilation problem we fixed last contained some dead code that I didn't remove, as I didn't want to be too intrusive. if it was my code, I would have removed those as well). Another completely unrelated topic: have you thought of using git instead of CVS? It would be so much simpler to contribute :) -- Bazsi - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/ ___ Libdbi-drivers-devel mailing list Libdbi-drivers-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libdbi-drivers-devel