Error getting client version info

[felipenoris]

Hi!

I took some time to upgrade Oracle.jl dependency from odpi v3.1.2 to the latest v4.1.

Everything worked flawless, except getting the client version info using dpiContext_getClientVersion.

After debugging a bit, I found a few things outdated in the docs:

• structure dpiVersionInfo fullVersionNum is now a uint32

• dpiContext_getClientVersion returns int instead of void

Even after these corrections, I can't get this function to work. This is what I'm getting from it:

Oracle.OraVersionInfo(-1020988144, 32627, -1021142744, 32627, 1065532090, 0x3c7d5648)

When I query the server version, which shares the same dpiVersionInfo struct, I get the expected result:

Oracle.OraVersionInfo(12, 1, 0, 2, 0, 0x4795cf08)

Can you check that dpiContext_getClientVersion is working correctly?

[kubo]

dpiContext_getClientVersion works fine for rust-oracle.

It looks that the context is not created correctly.
In https://github.com/felipenoris/Oracle.jl/blob/f18740a01a734b43f8c2e104e20a9d9ecd05c230/src/context.jl#L14

result = dpiContext_createWithParams(ORA_MAJOR_VERSION, ORA_MINOR_VERSION, context_handle_ref, error_info_ref)

dpiContext_createWithParams takes five arguments. However four arguments are passed.
EDITED: Four arguments are passed here.

[felipenoris]

@kubo thanks for the input! I'm effectively passing NULL for argument dpiContextCreateParams *params when calling the C funcion dpiContext_createWithParams. So I guess this is not a problem.

[kubo]

#include <stdio.h>
#include <string.h>
#include "dpi.h"

int main()
{
    dpiContext *context;
    dpiErrorInfo errinfo;
    dpiVersionInfo ver;

    dpiContext_createWithParams(DPI_MAJOR_VERSION, DPI_MINOR_VERSION, NULL, &context, &errinfo);
    dpiContext_getClientVersion(context, &ver);
    dpiContext_destroy(context);

    printf("1st: %d.%d.%d.%d.%d 0x%x\n", ver.versionNum, ver.releaseNum, ver.updateNum, ver.portReleaseNum, ver.portUpdateNum, ver.fullVersionNum);

    memset(&ver, 0, sizeof(ver));
    dpiContext_createWithParams(DPI_MAJOR_VERSION, DPI_MINOR_VERSION, NULL, &context, &errinfo);
    dpiContext_getClientVersion(context, &ver);
    dpiContext_destroy(context);

    printf("2nd: %d.%d.%d.%d.%d 0x%x\n", ver.versionNum, ver.releaseNum, ver.updateNum, ver.portReleaseNum, ver.portUpdateNum, ver.fullVersionNum);
}

This outputted:

1st: 21.1.0.0.0 0x7d3ab740
2nd: 1624216261.21951.-1.0.1645230224 0x55bf

[kubo]

This may be same problem with #153.
In #153, the versionInfo member of dpiContext points to unallocated memory and gets SEGV.
In this issue, it points to allocated memory and gets garbage.

[tgulacsi]

See #153 - ODPI does not support more than one context at the moment. Create one, and use that everywhere.

[felipenoris]

See #153 - ODPI does not support more than one context at the moment. Create one, and use that everywhere.

I think the code example uses one context at a time, since it destroys the first one before creating a new one.

This also looks like a regression, given that it worked fine on v3.1.2.

[tgulacsi]

I've been bitten by this again - I want to connect to two different databases from two different environments (TNS_ADMIN, ORACLE_PATH). As confDir and libDir is tied to the Context, I cannot accomplish this task now.

[anthony-tuininga]

@tgulacsi, are you suggesting that you could in the past? The environment variables (like TNS_ADMIN) are only read by the Oracle Client libraries the first time a connection is established. After that, it doesn't matter what values you provide before creating another connection -- they will be ignored. Can you clarify?

[tgulacsi]

No, I couldn't.

The following change (save ClientVersionInfo and reuse it) solves the SIGSEGV.
(I had to copy the whole struct as somewhere that *clientVersionInfo gets zeroed out).

But you're right, it still doesn't work, instead of SIGSEGV, I get "invalid username/password".

commit dd986efac7a629144b6eb581d3083fdbc0aabe57
Author: Tamás Gulácsi <[email protected]>
Date:   Tue Apr 13 18:19:13 2021 +0200

    Patch ODPI to save clientVersionInfo

diff --git a/odpi/src/dpiGlobal.c b/odpi/src/dpiGlobal.c
index 0a65366..50cd0af 100644
--- a/odpi/src/dpiGlobal.c
+++ b/odpi/src/dpiGlobal.c
@@ -46,7 +46,7 @@ static void *dpiGlobalEnvHandle = NULL;
 static void *dpiGlobalErrorHandle = NULL;
 static void *dpiGlobalThreadKey = NULL;
 static dpiErrorBuffer dpiGlobalErrorBuffer;
-static int dpiGlobalInitialized = 0;
+static dpiVersionInfo dpiGlobalClientVersionInfo;
 
 // a global mutex is used to ensure that only one thread is used to perform
 // initialization of ODPI-C
@@ -71,6 +71,7 @@ int dpiGlobal__ensureInitialized(const char *fnName,
         dpiContextCreateParams *params, dpiVersionInfo **clientVersionInfo,
         dpiError *error)
 {
+    dpiVersionInfo versionInfo;
     // initialize error buffer output to global error buffer structure; this is
     // the value that is used if an error takes place before the thread local
     // error structure can be returned
@@ -78,16 +79,21 @@ int dpiGlobal__ensureInitialized(const char *fnName,
     error->buffer = &dpiGlobalErrorBuffer;
     error->buffer->fnName = fnName;
 
+    if (dpiGlobalClientVersionInfo.versionNum != 0) {
+        versionInfo = dpiGlobalClientVersionInfo;
+        *clientVersionInfo = &versionInfo;
+    } else {
     // perform global initializations, if needed
-    if (!dpiGlobalInitialized) {
         dpiMutex__acquire(dpiGlobalMutex);
-        if (!dpiGlobalInitialized)
-            dpiGlobal__extendedInitialize(params, clientVersionInfo, error);
+        if (dpiGlobalClientVersionInfo.versionNum == 0) {
+            if (dpiGlobal__extendedInitialize(params, clientVersionInfo, error) == DPI_SUCCESS) {
+                dpiGlobalClientVersionInfo = *(*clientVersionInfo);
+            }
+        }
         dpiMutex__release(dpiGlobalMutex);
-        if (!dpiGlobalInitialized)
+        if (dpiGlobalClientVersionInfo.versionNum == 0)
             return DPI_FAILURE;
     }
-
     return dpiGlobal__getErrorBuffer(fnName, error);
 }
 
@@ -132,9 +138,6 @@ static int dpiGlobal__extendedInitialize(dpiContextCreateParams *params,
         return DPI_FAILURE;
     }
 
-    // mark library as fully initialized
-    dpiGlobalInitialized = 1;
-
     return DPI_SUCCESS;
 }
 
@@ -150,7 +153,7 @@ static void dpiGlobal__finalize(void)
     dpiError error;
 
     dpiMutex__acquire(dpiGlobalMutex);
-    dpiGlobalInitialized = 0;
+    dpiGlobalClientVersionInfo.versionNum = 0;
     error.buffer = &dpiGlobalErrorBuffer;
     if (dpiGlobalThreadKey) {
         dpiOci__threadKeyGet(dpiGlobalEnvHandle, dpiGlobalErrorHandle,
@@ -239,7 +242,7 @@ int dpiGlobal__initError(const char *fnName, dpiError *error)
 
     // check to see if global environment has been initialized; if not, no call
     // to dpiContext_createWithParams() was made successfully
-    if (!dpiGlobalInitialized)
+    if (dpiGlobalClientVersionInfo.versionNum == 0)
         return dpiError__set(error, "check context creation",
                 DPI_ERR_CONTEXT_NOT_CREATED);
 

[tgulacsi]

@anthony-tuininga , can you confirm that with the current state of OCI libraries, one cannot change libDir and configDir (ORACLE_PATH, TNS_ADMIN) once connected to a database?

That negative answer may also help, as I'll try to find an alternate solution, and not force this in one process.

[anthony-tuininga]

Yes, once you have connected to a database you cannot change the values of the library directory and configuration directory. To be more precise, once dpiGlobal__extendedInitialize() has run, the values are fixed and will not change. That occurs the first time you call dpiContext_createWithParams(). Take note that the documentation states this as well in the information for the parameter params. This could perhaps be strengthened. I'll see what @cjbj thinks.

I also see the problem with creating multiple contexts and the client version information. I'll address that and add a test case to ensure that the problem truly is resolved. Thanks for diagnosing the issue!

[anthony-tuininga]

I've just pushed changes to correct this issue and added a test case as well.

[tgulacsi]

Thanks, I can confirm those fixes work for me!

[felipenoris]

I can confirm it works also:

Oracle.OraVersionInfo(21, 1, 0, 0, 0, 2101000000)

[felipenoris]

@anthony-tuininga , maybe I missed something in the commit history, but I guess these should be fixed in the docs:

• structure dpiVersionInfo fullVersionNum is now a uint32

• dpiContext_getClientVersion returns int instead of void

[anthony-tuininga]

You are correct, @felipenoris! I think those were copy/paste errors that simply never got caught before. Thanks!

[anthony-tuininga]

ODPI-C 4.2 has been released which contains the patch for this issue. Thanks, everyone, for your help in uncovering this issue!