completed promise does not resolve after terminate

[Christilut]

My code is waiting for the completed promise, eg await this.pool.completed() but after using await this.pool.terminate(true), the completed promise still waits.
Is that intended? Shouldn't it either throw or resolve?

I can work around it with a second promise and using Promise.any() but that seems unintuitive.

[andywer]

Thanks for reporting! Need to look into it, but looks like a bug.

[andywer]

I gave it a look, but wasn't able to precisely pin the issue down. Is it possible that passing true to this.pool.completed() solves the issue (await this.pool.completed(true))?

The true makes the promise returned by pool.completed() resolve immediately in case that there are no actively running tasks, for instance if you just created some tasks, but they haven't started yet. By default pool.completed() would not resolve in that case, since otherwise a call to pool.completed() right after initial task creation would resolve immediately, before the pool got the chance to actually run them.

[Christilut]

Just tried await this.pool.completed(true) but it has the same problem.

This is my full code:

export class ThreadManager {
  private worker: Worker
  private pool: Pool<any>
  private _canceled: boolean = false
  private completed: boolean = false

  onProgress: IThreadManagerOnProgressFunction

  get canceled(): boolean {
    return this._canceled
  }

  constructor(worker: Worker, concurrency: number = -1) {
    if (worker.constructor.name !== 'WebWorker') throw new Error('invalid worker type, did you forget to import Worker from threads?')

    if (concurrency === -1) {
      concurrency = cpus().length
    }

    this.worker = worker

    this.pool = Pool(() => spawn(this.worker), {
      concurrency: 1,
      size: concurrency
    })

    EventBus.$on(EventBusEvents.Navigate, async () => {
      await this.cancel()
    })

    log.verbose(`created thread pool with ${concurrency} workers`)
  }

  async start(data: any[], workerArgs: any): Promise<any[]> {
    log.verbose(`started thread pool execution with ${data.length} messages`)

    const results: any[] = []
    let amountDone: number = 0

    for (const d of data) {
      this.pool.queue(async executor => { // eslint-disable-line
        const result = await executor(d, workerArgs)

        if (this.canceled) return

        amountDone++

        if (result) {
          results.push(result)
        }

        if (this.onProgress) {
          this.onProgress(amountDone / data.length)
        }
      })
    }

    await this.pool.completed() // TODO: on cancel

    this.completed = true // It never gets here when canceled...

    await this.pool.terminate(true)

    log.verbose('thread pool execution complete')

    return results
  }

  async cancel() {
    if (this.canceled || this.completed) return

    this._canceled = true

    await this.pool.terminate(true)

    log.verbose('thread pool execution canceled')
  }
}

[andywer]

Thanks for sharing the code! Definitely makes it easier. However, I don't see any obvious issue with it either.

So if you are only running it in the browser (?), try to set localStorage.setItem("DEBUG", "threads*") to enable debug logging. Maybe you will find some more helpful information from under threads.js' hood there.

[Christilut]

This is run in Electron, does that make a difference?

[andywer]

As long as threads.js run in the renderer process, it shouldn't make a difference.

[Christilut]

Just tried to get some debug output but neither the localStorage method nor the env variable method seems to do anything. I confirmed process.env.DEBUG is threads:* but I had no output.

[Christilut]

Sorry, it does log, I had verbose off in my console.

That error happens a lot until this:

[leopucci]

I was going to use it on the main process. It doesn´t work there?

As long as threads.js run in the renderer process, it shouldn't make a difference.

[andywer]

@leopucci It should also work in the main process. It will then just use a different implementation (node.js instead of web implementation) internally.

[leopucci]

Thanks Em sex, 10 de dez de 2021 17:49, Andy Wermke ***@***.***> escreveu:

…

@leopucci <https://github.com/leopucci> It should also work in the main process. It will then just use a different implementation (node.js instead of web implementation) internally. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#316 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJFZV4YZO2GM6KHDDZGC7WTUQJRXJANCNFSM4TM6P6UQ> .