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

Closing the client doesn't end the process sometimes #681

Closed
2 of 5 tasks
Zbizu opened this issue Dec 19, 2023 · 13 comments
Closed
2 of 5 tasks

Closing the client doesn't end the process sometimes #681

Zbizu opened this issue Dec 19, 2023 · 13 comments
Labels
Priority: High Represent a high impact in key areas of the base/user experience Priority: Medium This issue may be impactful and needs some attention. Status: Pending Test This PR or Issue requires more testing Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.

Comments

@Zbizu
Copy link
Contributor

Zbizu commented Dec 19, 2023

Priority

Medium

Area

  • Data
  • Source
  • Docker
  • Other

What happened?

Exiting the client leaves the process alive sometimes
obraz

I ran a debugger and it seems to be stuck on this while loop (the file belongs to openAL package)
obraz

Steps to reproduce

  1. login to some server
  2. walk around a bit
  3. ctrl + L
  4. press X button in the corner

Reproduction rate

High, but not always, estimated: ~40%

What OS are you seeing the problem on?

Windows

Code of Conduct

  • I agree to follow this project's Code of Conduct
@Zbizu Zbizu added the Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. label Dec 19, 2023
@github-actions github-actions bot added Priority: High Represent a high impact in key areas of the base/user experience Priority: Medium This issue may be impactful and needs some attention. Status: Pending Test This PR or Issue requires more testing labels Dec 19, 2023
@Zbizu
Copy link
Contributor Author

Zbizu commented Dec 19, 2023

after some digging I've found that this seems to be related to sounds in client

  • closing the client with music off keeps the thread hanging

  • closing the client with music on throws read access violation in some visual studio file
    obraz

@conde2
Copy link
Collaborator

conde2 commented Dec 19, 2023

image

You could check if this while is not stopping, and see what task is not making it to stop.

@Zbizu
Copy link
Contributor Author

Zbizu commented Dec 19, 2023

@conde2 I placed a breakpoint, the app does not reach it when it gets stuck
obraz

if it helps this is my terminal (connection is relatively stable for a god character, these are just missing packets from 13.20 protocol)

ERROR: ProtocolGame parse message exception (11353 bytes, 2239 unread, last opcode is 0x78 (120), prev opcode is 0x78 (120)): unknown container type 3
Packet has been saved to packet.log, you can use it to find what was wrong. (Protocol: 1320)
ERROR: ProtocolGame parse message exception (81 bytes, 68 unread, last opcode is 0xcd (205), prev opcode is 0xffffffff (-1)): unable to create item with invalid id 0
Packet has been saved to packet.log, you can use it to find what was wrong. (Protocol: 1320)
ERROR: ProtocolGame parse message exception (84 bytes, 63 unread, last opcode is 0x5d (93), prev opcode is 0xffffffff (-1)): unknown container type 3
Packet has been saved to packet.log, you can use it to find what was wrong. (Protocol: 1320)
ERROR: ProtocolGame parse message exception (3340 bytes, 2355 unread, last opcode is 0xe4 (228), prev opcode is 0xde (222)): InputMessage eof reached
Packet has been saved to packet.log, you can use it to find what was wrong. (Protocol: 1320)
ERROR: ProtocolGame parse message exception (53 bytes, 21 unread, last opcode is 0x78 (120), prev opcode is 0x78 (120)): unknown container type 3
Packet has been saved to packet.log, you can use it to find what was wrong. (Protocol: 1320)

and this mysterious X is in the bottom right corner of my UI
obraz

using most recent otc with no changes to the code and I didn't add/edit any modules

@conde2
Copy link
Collaborator

conde2 commented Dec 19, 2023

Which Tibia version are you using? I only use the engine (in 1098 version), not the modules so its going to be hard to help with it.

@Zbizu
Copy link
Contributor Author

Zbizu commented Dec 19, 2023

13.20 with a custom server engine (built for rl client though, this is first time I'm running OTC tests)

I managed to get 100% reproduction rate with this scenario:

  1. login
  2. walk to depot top floor
  3. walk to depot ground floor
  4. logout
  5. close the app (x button)

either it ends with read access violation or app gets stuck

@conde2
Copy link
Collaborator

conde2 commented Dec 19, 2023

I would recommend fixing the protocol errors first, I'm not sure if this can be related.
Could you check if there is any missing protocol parsing in otclient for 13.20?

@Zbizu
Copy link
Contributor Author

Zbizu commented Dec 19, 2023

quiver ammo count (the "new" implementation) seems to be missing, though I'm not sure if it's 13.20 or 13.21 change

this is what my server sends if the item is a quiver:

	if (it.type == ITEM_TYPE_CONTAINER || it.type == ITEM_TYPE_DEPOT) {
		const Container* container = item->getContainer();
		const Player* player = item->getHoldingPlayer();

		// assigned loot container icon
		if (container && player) {
			if (it.weaponType == WEAPON_QUIVER) {
				addByte(0x03); // flags: lootbag = true, quiver = true
				add<uint32_t>(player->getContainerFlags(container));
				add<uint32_t>(container->getAmmoCount());
			} else {
				addByte(0x01);
				add<uint32_t>(player->getContainerFlags(container));
			}
		} else {
			// no one holds the item, send no icon
			addByte(0x00);
		}
	}

I'm taking a break now. Will try without a quiver in one hour.
still, a Lua error shouldn't prevent the app from shutting itself down

@Zbizu
Copy link
Contributor Author

Zbizu commented Dec 19, 2023

update: I resolved protocol differences and I am no longer able to reproduce this problem, though this issue should be addressed as the end users will find it suspicious that it stays in process list after closing

on a side note that x in the bottom right corner is still visible 😅

@conde2
Copy link
Collaborator

conde2 commented Dec 19, 2023

Could you please open a pull request to fix the protocol error?

@SkullzOTS
Copy link
Collaborator

update: I resolved protocol differences and I am no longer able to reproduce this problem, though this issue should be addressed as the end users will find it suspicious that it stays in process list after closing

on a side note that x in the bottom right corner is still visible 😅

This "x" you refer to belongs to the gameinterface's extra panels, if you want to hide this, just go to the onExtraPanelVisibilityChange function in gameinterface.lua and find: v.checkbox:setVisible(true), change it to false

@SkullzOTS
Copy link
Collaborator

update: I resolved protocol differences and I am no longer able to reproduce this problem, though this issue should be addressed as the end users will find it suspicious that it stays in process list after closing

on a side note that x in the bottom right corner is still visible 😅

image

change to false

@Zbizu
Copy link
Contributor Author

Zbizu commented Dec 20, 2023

@conde2 I did quick rough edits just to get rid of errors, I'll open a draft once I clean it up.

@mehah
Copy link
Owner

mehah commented Dec 28, 2023

6fbbfd9

@mehah mehah closed this as completed Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High Represent a high impact in key areas of the base/user experience Priority: Medium This issue may be impactful and needs some attention. Status: Pending Test This PR or Issue requires more testing Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.
Projects
None yet
Development

No branches or pull requests

4 participants