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

cmdID should not be assigned in the cmd class constructor. Instead, assign cmdID after registering cmdtable by iterating through all commands in cmdtable #2686

Closed
chejinge opened this issue May 30, 2024 · 1 comment · Fixed by #2692
Labels
☢️ Bug Something isn't working

Comments

@chejinge
Copy link
Collaborator

chejinge commented May 30, 2024

Is this a regression?

Yes

Description

cmdID的赋值,不应该这样塞在cmd类的构造函数里,应该在注册完cmdtable以后, 遍历一次cmdtable里的所有命令,逐个给一下cmdid

Please provide a link to a minimal reproduction of the bug

cmd_table_manager依赖于cmd对象, 结果他又让cmd类去依赖cmd_table_manager,循环引用为其1,2是会有并发问题,data race

Screenshots or videos

并发问题是这样:每次一旦有新的命令对象被构造(调用CMD的构造函数),就会调用一次
image
这里面会对cmdID++
如果有多个线程同时在new一个新的cmd对象
这个函数就会被并发调用,但是这个被++的cmdID,是个普通类型, 不能并发, 而且更重要的是cmdID这个东西,不应该说每次有命令对象产生了,就要++,每个不同的命令一个ID

Please provide the version you discovered this bug in (check about page for version information)

No response

Anything else?

No response

@chejinge chejinge added the ☢️ Bug Something isn't working label May 30, 2024
@AlexStocks
Copy link
Collaborator

@gukaijie

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☢️ Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants