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

Make the "client connected" pre-check valid #626

Merged
merged 1 commit into from
May 20, 2024

Conversation

FJEagle
Copy link
Contributor

@FJEagle FJEagle commented May 20, 2024

This reverts commit 39e5ed4.

Why reverted?

The commit caused 2 problems:

Problem 1:

run in coroutine, make the "client connected" pre-check might not valid anymore when it runs, therefore the underlying sendNoWait 'client connected' check not matched causing the following exception:

image

Problem2:

run in coroutine, make the "sendToken" assignment-and-return invalid, it may always return null outside coroutine.

image

@FJEagle
Copy link
Contributor Author

FJEagle commented May 20, 2024

@hannesa2 could you help why check failed, I just revert the previous commit

@hannesa2
Copy link
Owner

yes, this sendToken is an issue. Thank you for pointing this out.
I try with #628 an other way to solve it

@FJEagle
Copy link
Contributor Author

FJEagle commented May 20, 2024

yes, this sendToken is an issue. Thank you for pointing this out. I try with #628 an other way to solve it

#628 should have solved problem 2 ,how about problem 1?

Run in coroutine might cause the underlying sendNoWait 'client connected' check not matched, results in throwing exception(32104), and then crash.

This exception seems not able to be caught in external business when call the publish method since it runs in coroutineScope

@hannesa2
Copy link
Owner

Maybe I don't really understand the issue exactly.
Please can you point me to

the 'client connected' pre-check

that we'll speak about the same

@FJEagle
Copy link
Contributor Author

FJEagle commented May 20, 2024

here are two screenshots:

the 'client connected' pre-check:
image

the underlying sendNoWait 'client connected' check and exception throw:
image

@hannesa2
Copy link
Owner

hannesa2 commented May 20, 2024

Please can you rebase to be able to merge ? I renamed confusing stuff before

@FJEagle
Copy link
Contributor Author

FJEagle commented May 20, 2024

ok, I'll do it, please wait for a while

@hannesa2
Copy link
Owner

Which version are you using ?

@FJEagle
Copy link
Contributor Author

FJEagle commented May 20, 2024

Which version are you using ?

4.3.beta2

@hannesa2 hannesa2 merged commit 96024a4 into hannesa2:master May 20, 2024
2 checks passed
@hannesa2 hannesa2 changed the title Revert "Less strict mode detected violations" Make the "client connected" pre-check valid May 20, 2024
@hannesa2
Copy link
Owner

@FJEagle
Copy link
Contributor Author

FJEagle commented May 20, 2024

https:/hannesa2/paho.mqtt.android/releases/tag/4.3.beta3

Thanks.

But it seems there are some errors in jitpack:

image

@hannesa2
Copy link
Owner

I re-triggered the build, now it's ok

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants