Medium/kew

Single-threaded based counters and Nashorn integration.

Closed this issue · 2 comments

Hi kew developers, your library is just great and ultra fast like you already said in the introduction.
I can imagine the library works perfectly in Node.js because it is based on a single-threaded environment, however the Oracle Nashorn JavaScript engine is a Java based (multi-threaded) runtime.

So I wanted to inform you that the existing code should be fixed to properly run inside Oracle Nashorn or Mozilla Rhino engines.

Next I paste my fixes, please consider to add a warning for Java developers.

Kind Regards,
Rolando "jWebSocket framework core developer&architect"

Source code:

    function allInternal(promises) {
        if (!promises.length)
            return resolve([])

        var outputs = []
        var finished = false
        var promise = new Promise()
        //var counter = promises.length
        var counter = new Packages.java.util.concurrent.atomic.AtomicInteger(promises.length)

        for (var i = 0; i < promises.length; i += 1) {
            if (!promises[i] || !isPromiseLike(promises[i])) {
                outputs[i] = promises[i]
                counter.decrementAndGet()
            } else {
                promises[i].then(replaceEl.bind(null, outputs, i))
                        .then(function decrementAllCounter() {
                            counter.decrementAndGet()
                            if (!finished && counter.get() === 0) {
                                finished = true
                                promise.resolve(outputs)
                            }
                        }, function onAllError(e) {
                            if (!finished) {
                                finished = true
                                promise.reject(e)
                            }
                        })
            }
        }

        if (counter.get() === 0 && !finished) {
            finished = true
            promise.resolve(outputs)
        }

        return promise
    }

    function allSettled(promises) {
        if (!Array.isArray(promises)) {
            throw Error('The input to "allSettled()" should be an array of Promise or values')
        }
        if (!promises.length)
            return resolve([])

        var outputs = []
        var promise = new Promise()
        var counter = new Packages.java.util.concurrent.atomic.AtomicInteger(promises.length)

        for (var i = 0; i < promises.length; i += 1) {
            if (!promises[i] || !isPromiseLike(promises[i])) {
                replaceElFulfilled(outputs, i, promises[i])
                if (counter.decrementAndGet() === 0)
                    promise.resolve(outputs)
            } else {
                promises[i]
                        .then(replaceElFulfilled.bind(null, outputs, i), replaceElRejected.bind(null, outputs, i))
                        .then(function () {
                            if (counter.decrementAndGet() === 0)
                                promise.resolve(outputs)
                        })
            }
        }

        return promise
    }
nicks commented

the changes you propose would break all other JS engines, so is a non-starter.

is there a machine-readable notation we can add to package.json to indicate that this code is not intended to be run on nashorn? maybe in the "engines" field? would be happy to add that.
https://docs.npmjs.com/files/package.json#engines

Hi nicks, thanks for your quick feedback. I need to say it was not my intention to suggest you modify the single-thread oriented library, excuses if that was the message you got.

In case you are interested in support Oracle high performance JavaScript engine, please consider my patch. Java users can "just" read an special warning note, something like this, or even better a Java oriented release.
In case I can contribute in any way, please let me know. Unfortunately the "engines" settings for NPM is not enough.

Best Regards,
Rolando