Prevent Lob Stream close event being emitted multiple times for error…

… conditions with Node 8

doc/api.md

@@ -4590,6 +4590,7 @@ At the conclusion of streaming into a Writeable Lob, the `close` event
 will occur.  It is recommended to put logic such as committing and
 releasing connections in this event (or after it occurs).  See
 [lobinsert2.js](https://github.com/oracle/node-oracledb/tree/master/examples/lobinsert2.js).
+It is also recommended for persistent LOBs not use the `finish` event handler for cleanup.
 
 ### <a name="lobinsertdiscussion"></a> 11.4 Using RETURNING INTO to Insert into LOBs
 

lib/lob.js

@@ -72,7 +72,9 @@ function Lob(iLob, opt, oracledb) {
     });
 
   if (this._autoCloseLob) {
-    this.once('finish', this._closeSync);
+    this.once('finish', function() {
+      this.close(function(err) {});
+    });
   }
 }
 
@@ -83,9 +85,7 @@ Lob.prototype._read = function() {
     function(err, str) {
       if (err) {
         if (self._autoCloseLob) {
-          // Ignore if any error occurs during close
-          // Emits 'close' event after closing LOB
-          self._closeSync();
+          self.emit('close');
         }
 
         self.emit('error', err);
@@ -94,17 +94,8 @@ Lob.prototype._read = function() {
 
       self.push(str);
 
-      if (self._autoCloseLob) {
-        if (!str) {
-          process.nextTick(function() {
-            err = self._closeSync(); // Emits 'close' event after closing LOB
-
-            if (err) {
-              self.emit('error', err);
-              return;
-            }
-          });
-        }
+      if (self._autoCloseLob && !str) {
+        self.emit('close');
       }
     }
   );
@@ -116,36 +107,14 @@ Lob.prototype._write = function(data, encoding, cb) {
   self.iLob.write(
     data,
     function(err) {
-      if (err) {
-        self._closeSync();   // Ignore if any error occurs during close
-        cb(err);
-        return;
+      if (err && self._autoCloseLob) {
+        self.emit('close');
       }
-
-      cb();
+      cb(err);
     }
   );
 };
 
-// This function will be deprecated in the future
-// This internal function used to close the LOB at the end of writable
-// stream in synchronus way to avoid race condition between this function and
-// application's listener function on 'finish' event.
-Lob.prototype._closeSync = function() {
-  var self = this;
-
-  if (self.iLob !== null) {
-    try {
-      self.iLob.release();
-    } catch(err) {
-      self.iLob = null;
-      return err;
-    }
-
-    self.emit('close');
-  }
-};
-
 Lob.prototype.close = function(closeCb) {
   var self = this;
 
@@ -160,7 +129,9 @@ Lob.prototype.close = function(closeCb) {
   }
 
   self.iLob.close(function(err) {
-    if (!err) {
+    if (err) {
+      self.emit('error', err);
+    } else  {
       self.emit('close');
     }
 

src/njs/src/njsIntLob.cpp

@@ -78,7 +78,6 @@ void njsILob::Init(Handle<Object> target)
     tpl->InstanceTemplate()->SetInternalFieldCount(1);
     tpl->SetClassName(Nan::New<v8::String>("ILob").ToLocalChecked());
 
-    Nan::SetPrototypeMethod(tpl, "release", Release);
     Nan::SetPrototypeMethod(tpl, "close", Close);
     Nan::SetPrototypeMethod(tpl, "read", Read);
     Nan::SetPrototypeMethod(tpl, "write", Write);
@@ -208,24 +207,6 @@ NAN_METHOD(njsILob::New)
 }
 
 
-//-----------------------------------------------------------------------------
-// njsILob::Release()
-//   Release the LOB handle immediately instead of when the object is garbage
-// collected.
-//
-// PARAMETERS
-//   - none
-//-----------------------------------------------------------------------------
-NAN_METHOD(njsILob::Release)
-{
-    njsILob *lob = (njsILob*) ValidateArgs(info, 0, 0);
-    if (!lob)
-        return;
-    dpiLob_release(lob->dpiLobHandle);
-    lob->dpiLobHandle = NULL;
-}
-
-
 //-----------------------------------------------------------------------------
 // njsILob::GetChunkSize()
 //   Get accessor of "chunkSize" property.
@@ -446,6 +427,16 @@ void njsILob::Async_Read(njsBaton *baton)
     if (dpiLob_readBytes(baton->dpiLobHandle, baton->lobOffset,
             baton->lobAmount, baton->bufferPtr, &baton->bufferSize) < 0)
         baton->GetDPIError();
+
+    // if an error occurs or the end of the LOB has been reached, and the LOB
+    // is marked as one that should be automatically closed, close and release
+    // it, ignoring any further errors that occur during the attempt to close
+    njsILob *lob = (njsILob*) baton->callingObj;
+    if (lob->isAutoClose && (!baton->bufferSize || !baton->error.empty())) {
+        dpiLob_close(lob->dpiLobHandle);
+        dpiLob_release(lob->dpiLobHandle);
+        lob->dpiLobHandle = NULL;
+    }
 }
 
 
@@ -566,8 +557,19 @@ NAN_METHOD(njsILob::Write)
 void njsILob::Async_Write(njsBaton *baton)
 {
     if (dpiLob_writeBytes(baton->dpiLobHandle, baton->lobOffset,
-            baton->bufferPtr, baton->bufferSize) < 0)
+            baton->bufferPtr, baton->bufferSize) < 0) {
         baton->GetDPIError();
+
+        // if an error occurs and the LOB is marked as one that should be
+        // automatically closed, close and release it, ignoring any further
+        // errors that occur during the attempt to close
+        njsILob *lob = (njsILob*) baton->callingObj;
+        if (lob->isAutoClose) {
+            dpiLob_close(lob->dpiLobHandle);
+            dpiLob_release(lob->dpiLobHandle);
+            lob->dpiLobHandle = NULL;
+        }
+    }
 }
 
 

src/njs/src/njsIntLob.h

@@ -125,7 +125,6 @@ class njsILob : public njsCommon {
     }
 
     static NAN_METHOD(New);
-    static NAN_METHOD(Release);
 
     // Read Method on ILob class
     static NAN_METHOD(Read);

test/blobDMLBindAsBuffer.js

@@ -555,7 +555,7 @@ describe('82.blobDMLBindAsBuffer.js', function() {
       );
     }); // 82.1.18
 
-    it.skip('82.1.19 Negative: RETURNING INTO with autocommit on', function(done) {
+    it('82.1.19 Negative: RETURNING INTO with autocommit on', function(done) {
       var id = insertID++;
       var sql = "INSERT INTO nodb_dml_blob_1 (id, blob) VALUES (:i, EMPTY_BLOB()) RETURNING blob INTO :lobbv";
       var inFileName = './test/tree.jpg';

test/clobDMLBindAsString.js

@@ -548,7 +548,7 @@ describe('81. clobDMLBindAsString.js', function() {
       ], done);
     }); // 81.1.18
 
-    it.skip('81.1.19 Negative: RETURNING INTO with autocommit on', function(done) {
+    it('81.1.19 Negative: RETURNING INTO with autocommit on', function(done) {
       var id = insertID++;
       var sql = "INSERT INTO nodb_dml_clob_1 (id, clob) VALUES (:i, EMPTY_CLOB()) RETURNING clob INTO :lobbv";
       var inFileName = './test/clobexample.txt';