vllm icon indicating copy to clipboard operation
vllm copied to clipboard

[BUG] RuntimeError: deque mutated during iteration in abort_seq_group

Open chenxu2048 opened this issue 1 year ago • 2 comments

Hi, we found that #2290 introduced a bug in function abort_seq_group. Attempting to mutate a deque during iteration will raise an exception.

Here is the sample code to reproduce the bug:

from vllm import LLMEngine, EngineArgs, SamplingParams
model = LLMEngine.from_engine_args(EngineArgs(model="meta-llama/Llama-2-7b-hf"))
model.add_request("0", "", SamplingParams())
model.add_request("1", "", SamplingParams())
model.abort_request(["0", "1"])

chenxu2048 avatar Jan 08 '24 04:01 chenxu2048

Nice catch, won't it be simpler to solve this by converting the deque to a list when iterating over it? I believe the code could stay the same as before except for:

for seq_group in list(state_queue):

NadavShmayo avatar Jan 08 '24 07:01 NadavShmayo

Nice catch, won't it be simpler to solve this by converting the deque to a list when iterating over it? I believe the code could stay the same as before except for:

for seq_group in list(state_queue):

Hi, @NadavShmayo. Thanks for your comments and nice clean solution.

But, here are two reasons why we chose an extra list:

  1. I think we should avoid removing elements while iterating over both List and Deque. The previous code with reversed() may work correctly, but it's dangerous.
  2. Using list(state_queue) to copy state_queue introduces overhead. In general, I think aborted_groups is shorter than the queue itself.

FYI

chenxu2048 avatar Jan 08 '24 08:01 chenxu2048