Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Queue with delayed jobs emits error when closing [BUG] #1413

Closed
misos1 opened this issue Aug 4, 2019 · 4 comments
Closed

Queue with delayed jobs emits error when closing [BUG] #1413

misos1 opened this issue Aug 4, 2019 · 4 comments
Labels

Comments

@misos1
Copy link

misos1 commented Aug 4, 2019

Description

In Queue.prototype.updateDelayTimer in closure .then(nextTimestamp => { is missing check whether is queue closing so it creates delayTimer even when is queue already closing/closed which after 5 seconds throws connection error. When is queue closing it is clearing that timeout but mentioned closure is sometimes called after closing so it creates new timeout which is nowhere cleared.

Minimal, Working Test code to reproduce the issue.

This bug may not appear every run of this test code. Cluster is used to increase probability. On different systems may be needed to increase number of workers (try with 100, it does not must be number of cores) or to run this multiple times to reproduce.

let cluster = require("cluster");
let Queue = require("bull");

if(cluster.isMaster)
{
	for(let i = 0; i < 10; i++) cluster.fork();
}
else
{
	let queue = new Queue("queue");
	queue.process(_job => console.log("started"));
	queue.on("error", err => console.log("queue error:", err));
	queue.add({}, { delay: 1000000 });
	queue.close();
	cluster.worker.disconnect(); // graceful shutdown
}

Possible output:

queue error: Error: Connection is closed.
    at Redis.sendCommand (node_modules/ioredis/built/redis.js:552:24)
    at Script.execute (node_modules/ioredis/built/script.js:23:34)
    at Redis.updateDelaySet (node_modules/ioredis/built/commander.js:152:24)
    at Object.updateDelaySet (node_modules/bull/lib/scripts.js:345:25)
    at Queue.updateDelayTimer (node_modules/bull/lib/queue.js:819:6)
    at Timeout._onTimeout (node_modules/bull/lib/queue.js:843:49)
    at listOnTimeout (internal/timers.js:531:17)
    at processTimers (internal/timers.js:475:7)

Bull version

3.10.0

Additional information

This change in bull/lib/queue.js Queue.prototype.updateDelayTimer seems fixes this bug:

     .then(nextTimestamp => {
+      if(this.closing) return;
       this.delayedTimestamp = nextTimestamp
@stale
Copy link

stale bot commented Jul 12, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added wontfix and removed wontfix labels Jul 12, 2021
@manast manast added the bug label Jul 13, 2021
@manast
Copy link
Member

manast commented Jul 13, 2021

@misos1 do not worry, I won't close issues that are legitimate bugs.

@todgru
Copy link

todgru commented Aug 7, 2023

This issue was fixed in #2535.

@misos1
Copy link
Author

misos1 commented Aug 7, 2023

Seems good now.

@misos1 misos1 closed this as completed Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants