Dynamsoft / barcode-reader-javascript

Dynamsoft Barcode Reader JavaScript SDK for package managers. PDF417, QR Code, DataMatrix, MaxiCode and more are supported.
https://www.dynamsoft.com/barcode-reader/sdk-javascript/
Other
168 stars 111 forks source link

throws "memory access out of bounds" under high load #106

Closed jkrnl closed 2 years ago

jkrnl commented 2 years ago

Hi There, Under some load (several concurrent calls from browser) I'm facing this wasm error message "memory access out of bounds"

node:internal/event_target:777 process.nextTick(() => { throw err; }); ^ RuntimeError: memory access out of bounds at wasm://wasm/01172d22:wasm-function[11931]:0x401f33 at wasm://wasm/01172d22:wasm-function[152]:0x8d1ec at wasm://wasm/01172d22:wasm-function[11901]:0x400804 at wasm://wasm/01172d22:wasm-function[341]:0xa9b73 at wasm://wasm/01172d22:wasm-function[340]:0xa9aa1 at wasm://wasm/01172d22:wasm-function[446]:0xad5d0 at wasm://wasm/01172d22:wasm-function[528]:0xb06f0 at wasm://wasm/01172d22:wasm-function[524]:0xb048a at wasm://wasm/01172d22:wasm-function[510]:0xaf8ca at wasm://wasm/01172d22:wasm-function[285]:0xa5803 Error: memory access out of bounds at /usr/src/app/node_modules/dynamsoft-javascript-barcode/dist/dbr.pure.js:11:21490 at Function. (/usr/src/app/node_modules/dynamsoft-javascript-barcode/dist/dbr.pure.js:11:14143) at Generator.next () at /usr/src/app/node_modules/dynamsoft-javascript-barcode/dist/dbr.pure.js:11:732 at new Promise () at d (/usr/src/app/node_modules/dynamsoft-javascript-barcode/dist/dbr.pure.js:11:477) at Worker.u._dbrWorker.onmessage (/usr/src/app/node_modules/dynamsoft-javascript-barcode/dist/dbr.pure.js:11:13166) at Worker.emit (node:events:390:28) at Worker.emit (node:domain:475:12) at MessagePort. (node:internal/worker:232:53) {"eventType":"error","log":{}} RuntimeError: memory access out of bounds at wasm://wasm/01172d22:wasm-function[240]:0x90770 at wasm://wasm/01172d22:wasm-function[291]:0xa7c63 at BarcodeReaderWasm$decodeFileInMemory [as decodeFileInMemory] (eval at qd (/usr/src/app/node_modules/dynamsoft-javascript-barcode/dist/dbr-8.8.0.node.wasm.js:92:242), :9:10) at /usr/src/app/node_modules/dynamsoft-javascript-barcode/dist/dbr-8.8.0.worker.js:108:48 at Generator.next () at b (/usr/src/app/node_modules/dynamsoft-javascript-barcode/dist/dbr-8.8.0.worker.js:21:73) Error: memory access out of bounds at /usr/src/app/node_modules/dynamsoft-javascript-barcode/dist/dbr.pure.js:11:21490 at Function. (/usr/src/app/node_modules/dynamsoft-javascript-barcode/dist/dbr.pure.js:11:14143) at Generator.next () at /usr/src/app/node_modules/dynamsoft-javascript-barcode/dist/dbr.pure.js:11:732 at new Promise () at d (/usr/src/app/node_modules/dynamsoft-javascript-barcode/dist/dbr.pure.js:11:477) at Worker.u._dbrWorker.onmessage (/usr/src/app/node_modules/dynamsoft-javascript-barcode/dist/dbr.pure.js:11:13166) at Worker.emit (node:events:390:28) at Worker.emit (node:domain:475:12) at MessagePort. (node:internal/worker:232:53) undefined

Node.js v17.1.0 Dynamsoft lib version 8.8.0 Docker base image:node:current-slim

Please cold you help me to solve it? Do I need to increase any wasm config setting maybe? Thank you

Keillion commented 2 years ago

To reproduce the problem, can you provide us browser info (e.g. chrome97 in android, FF95 in PC, Safari in iphoneX ios 15.1), and the min code?

Keillion commented 2 years ago

Oh, sorry, i know. You are the yesterday guy using nodejs as server side decoding. Not need browser info. I will do some test in node 17.

jkrnl commented 2 years ago

Seems I can overload it by 2-3 quick concurrent request right after (or around) the creatInstance call and probably when the 1st decode happens. When I keep it running after starup for a while before sending many request, it rarely goes to error. I have check node memory, it did not run out from free mem.

Keillion commented 2 years ago

Thanks very much. I can reproduce the problem in

https://github.com/dynamsoft-rd-0/dbrjs-mass-samples/tree/master/nodejs-memory-leak

Seems the memory goes up and never down.

I guess there is a buffer transfer bug. Something may not work same as browser.

If your problem only happen in first time decoding, refer create a temp dbr instance, so we can init license in https://github.com/dynamsoft-rd-0/dbrjs-mass-samples/blob/master/nodejs-memory-leak/app.js#L13

We need continue research it.

Keillion commented 2 years ago

Dbrjs currently only has one worker thread decoding. It will only process one task and only use one piece of memory. The other decoding tasks need waiting.

I find there can't be two decodingFile tasks, not know why.

Since the tasks must be queued. We can add queue ahead.

Here I use express as example. If you use koa, the code may look more pretty.

Tested, works well.

jkrnl commented 2 years ago

Thank you! I'll try it. Is there any way to improve Dbrjs, to make our app scale ? This queueing limits us in terms of troughput/performance What is with the .net(C#) barcode library? Do we have this 1 thread limitation there also? we may consider using different language/platform if needed

Keillion commented 2 years ago

Yes, we do have C# version, which not have the one thread limitation.

) If you use server side decoding, it depends on the number of your machine's cpu cores.

It's must efficient when the core number equals the working thread number. We are not provide the thread pool since their are many mature solutions.

And if you need high concurrency, another way is using load balancing. So the single server's performance is not important. You can have several one-core machines, using any language you like, best to have only one thread in each machine.

) Another way use browser side decoding.

jkrnl commented 2 years ago

Thanks for th updates. So memory leak is no longer an issue once we use queueing or do you still checking that meanwhile?

jkrnl commented 2 years ago

Another question: in the node.js component I'd avoid creating new reader instance on each incoming request. Do you see any issue to keep a singleton reader instance and call decodes only request-by-request (queueing will be there)?

Keillion commented 2 years ago

Thanks for th updates. So memory leak is no longer an issue once we use que

I will keep look at the issue. By design the queue is inside. Users not need to know. The outside queue is only a walkaround.

And in the future we may support multiple working threads.

Keillion commented 2 years ago

Another question: in the node.js component I'd avoid creating new reader instance on each incoming request. Do you see any issue to keep a singleton reader instance and call decodes only request-by-request (queueing will be there)?

It's ok. The performance will be slightly improved.

jkrnl commented 2 years ago

Please, could you help me when can we get the new fixed version?

Keillion commented 2 years ago

One of our members has been working on this for a week. No root problem found.

Just 2 hours ago, we found that with a different version of the wasm compiler, the problem magically disappeared. We need to continue to run the test cases to make sure that this change has no other adverse effects.

We will release an internal version tomorrow, Friday.

jkrnl commented 2 years ago

Perfect, thank you!

Keillion commented 2 years ago

"keillion-dynamsoft-javascript-barcode": "0.20220121160720.0" problem resolved.

My test code. I open several index.html to do loop decoding. https://github.com/dynamsoft-rd-0/dbrjs-mass-samples/tree/master/nodejs-memory-leak

Our testing team has also done regular testing.

jkrnl commented 2 years ago

Thanks lot, let's check it here also!

Keillion commented 2 years ago

dynamsoft-node-barcode@8.8.7 fixed https://www.npmjs.com/package/dynamsoft-node-barcode