Repository: mholt/PapaParse (js)

Issue:
Pause and resume (In a quick succession) gets you  lot of exceptions and an infinite loop
Doing pause and resume (In a quick succession) on demo gets you  lot of exceptions :- 

```
papaparse.js:733 Uncaught DOMException: Failed to execute 'readAsText' on 'FileReader': The object is already busy reading Blobs.
    at FileStreamer._readChunk (https://www.papaparse.com/resources/js/papaparse.js:733:21)
    at FileStreamer._nextChunk (https://www.papaparse.com/resources/js/papaparse.js:722:10)
    at FileStreamer.ChunkStreamer.parseChunk (https://www.papaparse.com/resources/js/papaparse.js:538:10)
    at FileStreamer._chunkLoaded (https://www.papaparse.com/resources/js/papaparse.js:743:9)
    at FileReader.<anonymous> (https://www.papaparse.com/resources/js/papaparse.js:1800:25)
FileStreamer._readChunk	@	papaparse.js:733
FileStreamer._nextChunk	@	papaparse.js:722
ChunkStreamer.parseChunk	@	papaparse.js:538
FileStreamer._chunkLoaded	@	papaparse.js:743
(anonymous)	@	papaparse.js:1800
load (async)		
FileStreamer.stream	@	papaparse.js:710
CsvToJson	@	papaparse.js:265
parseNextFile	@	papaparse.js:163
$.fn.parse	@	papaparse.js:113
(anonymous)	@	demo.js:103
dispatch	@	jquery.min.js:3
r.handle	@	jquery.min.js:3
```
And if you choose to handle them, like this :- 
```
function stepFn(results, parser)
{
	stepped++;
	if (results)
	{
		parser.pause();
		try{
		parser.resume();
		} catch (e) {
			console.log(e);
		}
		if (results.data){
			array2 = array2.concat(results.data); // Just to check the data
			rowCount += results.data.length;
		}
		if (results.errors)
		{
			errorCount += results.errors.length;
			firstError = firstError || results.errors[0];
		}
	}
}
```
Then you get into an infinite loop with the same data.

It appears that resume should be waiting for previous pause to halt the parsing before continuing again.
It appears this wasn't the case before since Issue #321 talks about doing this to produce another bug (Cursor related).
I am using :- 
`Papa.LocalChunkSize = 10485`
Otherwise you get :- 
```
RangeError: Maximum call stack size exceeded
    at FileStreamer.ChunkStreamer.parseChunk (papaparse.js:491)
    at ParserHandle.resume (papaparse.js:1078)
    at stepFn (demo.js:212)
    at ParserHandle._config.step (papaparse.js:1017)
    at doStep (papaparse.js:1651)
    at Parser.parse (papaparse.js:1561)
    at ParserHandle.parse (papaparse.js:1058)
    at FileStreamer.ChunkStreamer.parseChunk (papaparse.js:489)
    at ParserHandle.resume (papaparse.js:1078)
    at stepFn (demo.js:212)
```

Pull request:
BugFix #636 Pause and resume (In a quick succession) gets you lot of exceptions and an infinite loop
WIP: BugFix #636 Pause and resume (In a quick succession) gets you lot of exceptions and an infinite loop

So my plan is to have a logic of this kind for all the Streamers (Since ChunkStreamer is common in all), what do you feel about it? Any different ideas?

Patch:
diff --git a/papaparse.js b/papaparse.js
index 70c4edc..6e68a52 100755
--- a/papaparse.js
+++ b/papaparse.js
@@ -473,6 +473,7 @@ License: MIT
 		this._handle = null;
 		this._finished = false;
 		this._completed = false;
+		this._halted = false;
 		this._input = null;
 		this._baseIndex = 0;
 		this._partialLine = '';
@@ -497,6 +498,7 @@ License: MIT
 					chunk = modifiedChunk;
 			}
 			this.isFirstChunk = false;
+			this._halted = false;
 
 			// Rejoin the line we likely just split in two by chunking the file
 			var aggregate = this._partialLine + chunk;
@@ -504,8 +506,10 @@ License: MIT
 
 			var results = this._handle.parse(aggregate, this._baseIndex, !this._finished);
 
-			if (this._handle.paused() || this._handle.aborted())
+			if (this._handle.paused() || this._handle.aborted()) {
+				this._halted = true;
 				return;
+			}
 
 			var lastIndex = results.meta.cursor;
 
@@ -531,8 +535,10 @@ License: MIT
 			else if (isFunction(this._config.chunk) && !isFakeChunk)
 			{
 				this._config.chunk(results, this._handle);
-				if (this._handle.paused() || this._handle.aborted())
+				if (this._handle.paused() || this._handle.aborted()) {
+					this._halted = true;
 					return;
+				}
 				results = undefined;
 				this._completeResults = undefined;
 			}
@@ -992,7 +998,6 @@ License: MIT
 		// One goal is to minimize the use of regular expressions...
 		var FLOAT = /^\s*-?(\d*\.?\d+|\d+\.?\d*)(e[-+]?\d+)?\s*$/i;
 		var ISO_DATE = /(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d\.\d+([+-][0-2]\d:[0-5]\d|Z))|(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d([+-][0-2]\d:[0-5]\d|Z))|(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d([+-][0-2]\d:[0-5]\d|Z))/;
-
 		var self = this;
 		var _stepCounter = 0;	// Number of times step was called (number of rows parsed)
 		var _rowCounter = 0;	// Number of rows that have been parsed so far
@@ -1089,8 +1094,14 @@ License: MIT
 
 		this.resume = function()
 		{
-			_paused = false;
-			self.streamer.parseChunk(_input, true);
+			if(self.streamer._halted) {
+				_paused = false;
+				self.streamer.parseChunk(_input, true);
+			} else {
+				// Bugfix: #636 In case the processing hasn't halted yet
+				// wait for it to halt in order to resume
+				setTimeout(this.resume, 3);
+			}
 		};
 
 		this.aborted = function()


Test patch:
diff --git a/tests/node-tests.js b/tests/node-tests.js
index fe671df..a067bfe 100644
--- a/tests/node-tests.js
+++ b/tests/node-tests.js
@@ -41,6 +41,42 @@ describe('PapaParse', function() {
 		assertLongSampleParsedCorrectly(Papa.parse(longSampleRawCsv));
 	});
 
+	it('Pause and resume works (Regression Test for Bug #636)', function(done) {
+		this.timeout(30000);
+		var mod200Rows = [
+			["Etiam a dolor vitae est vestibulum","84","DEF"],
+			["Etiam a dolor vitae est vestibulum","84","DEF"],
+			["Lorem ipsum dolor sit","42","ABC"],
+			["Etiam a dolor vitae est vestibulum","84","DEF"],
+			["Etiam a dolor vitae est vestibulum","84"],
+			["Lorem ipsum dolor sit","42","ABC"],
+			["Etiam a dolor vitae est vestibulum","84","DEF"],
+			["Etiam a dolor vitae est vestibulum","84","DEF"],
+			["Lorem ipsum dolor sit","42","ABC"],
+			["Lorem ipsum dolor sit","42"]
+		];
+		var stepped = 0;
+		var dataRows = [];
+		Papa.parse(fs.createReadStream(__dirname + '/verylong-sample.csv'), {
+			step: function(results, parser) {
+				stepped++;
+				if (results)
+				{
+					parser.pause();
+					parser.resume();
+					if (results.data && stepped % 200 === 0) {
+						dataRows.push(results.data);
+					}
+				}
+			},
+			complete: function() {
+				assert.strictEqual(2001, stepped);
+				assert.deepEqual(mod200Rows, dataRows);
+				done();
+			}
+		});
+	});
+
 	it('asynchronously parsed CSV should be correctly parsed', function(done) {
 		Papa.parse(longSampleRawCsv, {
 			complete: function(parsedCsv) {
diff --git a/tests/test-cases.js b/tests/test-cases.js
index 07b8a53..d9dd459 100644
--- a/tests/test-cases.js
+++ b/tests/test-cases.js
@@ -1780,6 +1780,49 @@ describe('Unparse Tests', function() {
 
 
 var CUSTOM_TESTS = [
+	{
+		description: "Pause and resume works (Regression Test for Bug #636)",
+		disabled: !XHR_ENABLED,
+		timeout: 30000,
+		expected: [2001, [
+			["Etiam a dolor vitae est vestibulum","84","DEF"],
+			["Etiam a dolor vitae est vestibulum","84","DEF"],
+			["Lorem ipsum dolor sit","42","ABC"],
+			["Etiam a dolor vitae est vestibulum","84","DEF"],
+			["Etiam a dolor vitae est vestibulum","84"],
+			["Lorem ipsum dolor sit","42","ABC"],
+			["Etiam a dolor vitae est vestibulum","84","DEF"],
+			["Etiam a dolor vitae est vestibulum","84","DEF"],
+			["Lorem ipsum dolor sit","42","ABC"],
+			["Lorem ipsum dolor sit","42"]
+		], 0],
+		run: function(callback) {
+			var stepped = 0;
+			var dataRows = [];
+			var errorCount = 0;
+			var output = [];
+			Papa.parse(BASE_PATH + "verylong-sample.csv", {
+				download: true,
+				step: function(results, parser) {
+					stepped++;
+					if (results)
+					{
+						parser.pause();
+						parser.resume();
+						if (results.data && stepped % 200 === 0) {
+							dataRows.push(results.data);
+						}
+					}
+				},
+				complete: function() {
+					output.push(stepped);
+					output.push(dataRows);
+					output.push(errorCount);
+					callback(output);
+				}
+			});
+		}
+	},
 	{
 		description: "Complete is called with all results if neither step nor chunk is defined",
 		expected: [['A', 'b', 'c'], ['d', 'E', 'f'], ['G', 'h', 'i']],
@@ -2263,6 +2306,9 @@ var CUSTOM_TESTS = [
 describe('Custom Tests', function() {
 	function generateTest(test) {
 		(test.disabled ? it.skip : it)(test.description, function(done) {
+			if(test.timeout) {
+				this.timeout(test.timeout);
+			}
 			test.run(function(actual) {
 				assert.deepEqual(JSON.stringify(actual), JSON.stringify(test.expected));
 				done();