[ruby-oci8-commit] [547] trunk/ruby-oci8: Fix SEGV when a temporary LOB is freed when OCILobRead returns OCI_NEED_DATA .

nobody at rubyforge.org nobody at rubyforge.org
Sun Dec 23 11:47:17 UTC 2012


Revision: 547
Author:   kubo
Date:     2012-12-23 11:47:16 +0000 (Sun, 23 Dec 2012)
Log Message:
-----------
Fix SEGV when a temporary LOB is freed when OCILobRead returns OCI_NEED_DATA.
(github issue #20 reported by Edgars Beigarts)

Modified Paths:
--------------
    trunk/ruby-oci8/ChangeLog
    trunk/ruby-oci8/ext/oci8/lob.c
    trunk/ruby-oci8/ext/oci8/oci8.h
    trunk/ruby-oci8/ext/oci8/oci8lib.c
    trunk/ruby-oci8/test/test_clob.rb

Modified: trunk/ruby-oci8/ChangeLog
===================================================================
--- trunk/ruby-oci8/ChangeLog	2012-12-18 12:12:11 UTC (rev 546)
+++ trunk/ruby-oci8/ChangeLog	2012-12-23 11:47:16 UTC (rev 547)
@@ -1,3 +1,8 @@
+2012-12-23  KUBO Takehiro  <kubo at jiubao.org>
+	* ext/oci8/lob.c, ext/oci8/oci8.h, ext/oci8/oci8lib.c, test/test_clob.rb:
+	    fix SEGV when a temporary LOB is freed when OCILobRead returns OCI_NEED_DATA.
+	    (github issue #20 reported by Edgars Beigarts)
+
 2012-12-18  KUBO Takehiro  <kubo at jiubao.org>
 	* ext/oci8/error.c, lib/oci8/oci8.rb: change the OCIError constructor to
 	    accept an Oracle error code as the first parameter and create a message

Modified: trunk/ruby-oci8/ext/oci8/lob.c
===================================================================
--- trunk/ruby-oci8/ext/oci8/lob.c	2012-12-18 12:12:11 UTC (rev 546)
+++ trunk/ruby-oci8/ext/oci8/lob.c	2012-12-23 11:47:16 UTC (rev 547)
@@ -554,12 +554,24 @@
         /* initialize buf in zeros everytime to check a nul characters. */
         memset(buf, 0, sizeof(buf));
         rv = OCILobRead_nb(svcctx, svcctx->base.hp.svc, oci8_errhp, lob->base.hp.lob, &amt, lob->pos + 1, buf, sizeof(buf), NULL, NULL, 0, lob->csfrm);
-        if (rv == OCI_ERROR && oci8_get_error_code(oci8_errhp) == 22289) {
-            /* ORA-22289: cannot perform FILEREAD operation on an unopened file or LOB */
-            if (lob->state == S_BFILE_CLOSE)
-                continue;
-        }
-        if (rv != OCI_SUCCESS && rv != OCI_NEED_DATA) {
+        svcctx->suppress_free_temp_lobs = 0;
+        switch (rv) {
+        case OCI_SUCCESS:
+            break;
+        case OCI_NEED_DATA:
+            /* prevent OCILobFreeTemporary() from being called.
+             * See: https://github.com/kubo/ruby-oci8/issues/20
+             */
+            svcctx->suppress_free_temp_lobs = 1;
+            break;
+        case OCI_ERROR:
+            if (oci8_get_error_code(oci8_errhp) == 22289) {
+                /* ORA-22289: cannot perform FILEREAD operation on an unopened file or LOB */
+                if (lob->state == S_BFILE_CLOSE)
+                    continue;
+            }
+            /* FALLTHROUGH */
+        default:
             chker2(rv, &svcctx->base);
         }
 

Modified: trunk/ruby-oci8/ext/oci8/oci8.h
===================================================================
--- trunk/ruby-oci8/ext/oci8/oci8.h	2012-12-18 12:12:11 UTC (rev 546)
+++ trunk/ruby-oci8/ext/oci8/oci8.h	2012-12-23 11:47:16 UTC (rev 547)
@@ -361,6 +361,7 @@
     rb_pid_t pid;
     unsigned char state;
     char is_autocommit;
+    char suppress_free_temp_lobs;
 #ifdef NATIVE_THREAD_WITH_GVL
     char non_blocking;
 #endif

Modified: trunk/ruby-oci8/ext/oci8/oci8lib.c
===================================================================
--- trunk/ruby-oci8/ext/oci8/oci8lib.c	2012-12-18 12:12:11 UTC (rev 546)
+++ trunk/ruby-oci8/ext/oci8/oci8lib.c	2012-12-23 11:47:16 UTC (rev 547)
@@ -229,44 +229,45 @@
 /* ruby 1.9 */
 sword oci8_call_without_gvl(oci8_svcctx_t *svcctx, void *(*func)(void *), void *data)
 {
-    oci8_temp_lob_t *lob;
     OCIError *errhp = oci8_errhp;
 
     if (!NIL_P(svcctx->executing_thread)) {
         rb_raise(rb_eRuntimeError /* FIXME */, "executing in another thread");
     }
 
-    lob = svcctx->temp_lobs;
-    while (lob != NULL) {
-        oci8_temp_lob_t *lob_next = lob->next;
+    if (!svcctx->suppress_free_temp_lobs) {
+        oci8_temp_lob_t *lob = svcctx->temp_lobs;
+        while (lob != NULL) {
+            oci8_temp_lob_t *lob_next = lob->next;
 
-        if (svcctx->non_blocking) {
-            free_temp_lob_arg_t arg;
-            sword rv;
+            if (svcctx->non_blocking) {
+                free_temp_lob_arg_t arg;
+                sword rv;
 
-            arg.svcctx = svcctx;
-            arg.svchp = svcctx->base.hp.svc;
-            arg.errhp = errhp;
-            arg.lob = lob->lob;
+                arg.svcctx = svcctx;
+                arg.svchp = svcctx->base.hp.svc;
+                arg.errhp = errhp;
+                arg.lob = lob->lob;
 
-            svcctx->executing_thread = rb_thread_current();
+                svcctx->executing_thread = rb_thread_current();
 #ifdef HAVE_RB_THREAD_CALL_WITHOUT_GVL
-            rv = (sword)(VALUE)rb_thread_call_without_gvl(free_temp_lob, &arg, oci8_unblock_func, svcctx);
+                rv = (sword)(VALUE)rb_thread_call_without_gvl(free_temp_lob, &arg, oci8_unblock_func, svcctx);
 #else
-            rv = (sword)rb_thread_blocking_region((VALUE(*)(void*))free_temp_lob, &arg, oci8_unblock_func, svcctx);
+                rv = (sword)rb_thread_blocking_region((VALUE(*)(void*))free_temp_lob, &arg, oci8_unblock_func, svcctx);
 #endif
-            if (rv == OCI_ERROR) {
-                if (oci8_get_error_code(errhp) == 1013) {
-                    rb_raise(eOCIBreak, "Canceled by user request.");
+                if (rv == OCI_ERROR) {
+                    if (oci8_get_error_code(errhp) == 1013) {
+                        rb_raise(eOCIBreak, "Canceled by user request.");
+                    }
                 }
+            } else {
+                OCILobFreeTemporary(svcctx->base.hp.svc, errhp, lob->lob);
             }
-        } else {
-            OCILobFreeTemporary(svcctx->base.hp.svc, errhp, lob->lob);
+            OCIDescriptorFree(lob->lob, OCI_DTYPE_LOB);
+
+            xfree(lob);
+            svcctx->temp_lobs = lob = lob_next;
         }
-        OCIDescriptorFree(lob->lob, OCI_DTYPE_LOB);
-
-        xfree(lob);
-        svcctx->temp_lobs = lob = lob_next;
     }
 
     if (svcctx->non_blocking) {

Modified: trunk/ruby-oci8/test/test_clob.rb
===================================================================
--- trunk/ruby-oci8/test/test_clob.rb	2012-12-18 12:12:11 UTC (rev 546)
+++ trunk/ruby-oci8/test/test_clob.rb	2012-12-23 11:47:16 UTC (rev 547)
@@ -75,6 +75,15 @@
     lob.close
   end
 
+  # https://github.com/kubo/ruby-oci8/issues/20
+  def test_github_issue_20
+    lob1 = OCI8::CLOB.new(@conn, ' '  * (1024 * 1024))
+    lob2 = OCI8::CLOB.new(@conn, ' '  * (128 * 1024 * 1024))
+
+    lob1 = nil  # lob1's value will be freed in GC.
+    lob2.read   # GC must run here to reproduce the issue.
+  end
+
   def teardown
     drop_table('test_table')
     @conn.logoff



More information about the ruby-oci8-commit mailing list