Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

OCI_THREADED prevents crash with 11g libraries #2

Merged
merged 3 commits into from over 2 years ago

3 participants

Jonathan Wolfe Dossy Shiobara Andrew Steets
Jonathan Wolfe

See Issue 1 on aolserver/nsoracle.

Included in these commits is a fix to a leak in free_fetch_buffers - it was reusing an outer loop variable in an inner loop when freeing LOB descriptors, however I think Ns_OracleFlush actually handles this before free_fetch_buffers ever gets to it, and Ns_OracleFlush properly uses a new loop variable there. Can't hurt to fix the leak at least while that function still exists in the driver - I don't have a test (yet) to show that it can be removed entirely because it's superseded by the newer flush function.

Andrew Steets

Were you getting crashes sporadically throughout the server or was it crashing specifically in OCIEnvCreate? I had noticed our servers crashing in OCIEnvCreate when there were two simulaneous calls to OCIEnvCreate from different threads. That to me seemed to indicate that OCIEnvCreate itself was not threadsafe. I wrapped the call to OCIEnvCreate in a mutex and put further investigation on my TODO list. It made the crashing stop in my case, and I unfortunately never got around to doing more investigation.

After re-reading some OCI docs:
http://download.oracle.com/docs/cd/B14117_01/appdev.101/b10779/oci09adv.htm#469037
http://download.oracle.com/docs/cd/E11882_01/appdev.112/e10646/oci16rel001.htm#i556149

I think the previous mode parameter OCI_ENV_NO_MUTEX | OCI_DEFAULT is nonsensical. It looks like the two options are mutually exclusive. I believe the correct mode argument is probably OCI_THREADED | OCI_ENV_NO_MUTEX. The reason is that an environment handle is associated with a connection, and a connection is only held by one thread at a time, so all calls to a single environment handle (or any OCI handle) are serialized due to the nature of the architecture.

Jonathan Wolfe
jwolfe commented

You're absolutely right, each environment handle that's created here is the result of an ns_db open call, and as such OCI_ENV_NO_MUTEX should be added to OCI_THREADED because there shouldn't ever be a connection held by more than one thread as you note. Your manual mutex around OCIEnvCreate likely had the same effect.

The crashes I saw were in pthread calls in a test file that just creates 1000 concurrent environments in separate threads. I didn't spend a ton of time to get a better stack trace, but I know that changing this one flag in OCIEnvCreate caused the test to go from a crash every time to stable (and it's been stable in production ever since as well).

Dossy Shiobara
Collaborator

As I have no way of testing nsoracle, I'm going to trust the two of you have fully tested the change and since there has been no further discussion, there are no other needed changes before I merge this pull request?

Jonathan Wolfe

I don't know of any other changes necessary, I think it's safe to merge.

Dossy Shiobara
Collaborator

That's enough for me. Thanks! ;-)

Dossy Shiobara dossy merged commit 8e10840 into from
Dossy Shiobara dossy closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 4 additions and 3 deletions. Show diff stats Hide diff stats

  1. +4 3 nsoracle.c
7 nsoracle.c
@@ -2922,7 +2922,7 @@ Ns_OracleOpenDb (Ns_DbHandle *dbh)
2922 2922 dbh->connection = connection;
2923 2923
2924 2924 oci_status = OCIEnvCreate(&connection->env,
2925   - OCI_ENV_NO_MUTEX|OCI_DEFAULT,
  2925 + OCI_THREADED|OCI_ENV_NO_MUTEX,
2926 2926 NULL,
2927 2927 Ns_OracleMalloc,
2928 2928 Ns_OracleRealloc,
@@ -4571,6 +4571,7 @@ free_fetch_buffers(ora_connection_t * connection)
4571 4571 if (connection != NULL && connection->fetch_buffers != NULL) {
4572 4572 Ns_DbHandle *dbh = connection->dbh;
4573 4573 int i;
  4574 + int j;
4574 4575 oci_status_t oci_status;
4575 4576
4576 4577 for (i = 0; i < connection->n_columns; i++) {
@@ -4606,8 +4607,8 @@ free_fetch_buffers(ora_connection_t * connection)
4606 4607 }
4607 4608
4608 4609 if (fetchbuf->lobs != 0) {
4609   - for (i = 0; i < fetchbuf->n_rows; i++) {
4610   - oci_status = OCIDescriptorFree(fetchbuf->lobs[i],
  4610 + for (j = 0; j < fetchbuf->n_rows; j++) {
  4611 + oci_status = OCIDescriptorFree(fetchbuf->lobs[j],
4611 4612 OCI_DTYPE_LOB);
4612 4613 oci_error_p(lexpos(), dbh, "OCIDescriptorFree", 0,
4613 4614 oci_status);

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.