Skip to content

Conversation

Junglee-Faisal
Copy link
Contributor

Screenshot 2025-01-09 at 4 16 39 PM

It stops sending the result twice for the same method which fixes the above crash.

@Junglee-Faisal
Copy link
Contributor Author

Junglee-Faisal commented Jan 15, 2025

This PR address an issue in MethodResultWrapper.java that was causing an Application Not Responding (ANR) error. This fix ensures better stability and performance for all users of the package.

Could you kindly review the changes at your earliest convenience? Given the impact of this issue, a quick merge would be highly beneficial to the community. Please let me know if any adjustments are needed.

@RodrigoSMarques RodrigoSMarques changed the base branch from master to dev January 15, 2025 13:45
@RodrigoSMarques
Copy link
Owner

Thanks for the contribution. I will analyze the PR

@RodrigoSMarques
Copy link
Owner

@Junglee-Faisal

Have you tested this code?

A quick look at the code shows that on the first run the value will be set to "True" and it will never return "False".

@Junglee-Faisal
Copy link
Contributor Author

@Junglee-Faisal

Have you tested this code?

A quick look at the code shows that on the first run the value will be set to "True" and it will never return "False".

Hi @RodrigoSMarques
Thanks for the response

The changes are well-implemented and effectively prevent sending the result multiple times for the same method. The checkNotCalled method ensures that the flag is set to true the first time it is called, preventing subsequent calls from proceeding. This addresses the issue of sending the result multiple times.

@RodrigoSMarques
Copy link
Owner

Hi @RodrigoSMarques Thanks for the response

The changes are well-implemented and effectively prevent sending the result multiple times for the same method. The checkNotCalled method ensures that the flag is set to true the first time it is called, preventing subsequent calls from proceeding. This addresses the issue of sending the result multiple times.

Ok. But the result is strange.

The variable called is never set back to false and therefore all the other methods are never executed, since called is set to true.

For example:

I opened the application, called at startup = false. The processing continues and called = true.

The application returned empty BUO data, since I did not click on any link when opening the application.

If I put the application in the background and click on a link, the application opens, but it does not return the data, since called = true and the response does not return to the application.

@Junglee-Faisal
Copy link
Contributor Author

Hi @RodrigoSMarques Thanks for the response
The changes are well-implemented and effectively prevent sending the result multiple times for the same method. The checkNotCalled method ensures that the flag is set to true the first time it is called, preventing subsequent calls from proceeding. This addresses the issue of sending the result multiple times.

Ok. But the result is strange.

The variable called is never set back to false and therefore all the other methods are never executed, since called is set to true.

For example:

I opened the application, called at startup = false. The processing continues and called = true.

The application returned empty BUO data, since I did not click on any link when opening the application.

If I put the application in the background and click on a link, the application opens, but it does not return the data, since called = true and the response does not return to the application.

Hi @RodrigoSMarques Thanks for the response
The changes are well-implemented and effectively prevent sending the result multiple times for the same method. The checkNotCalled method ensures that the flag is set to true the first time it is called, preventing subsequent calls from proceeding. This addresses the issue of sending the result multiple times.

Ok. But the result is strange.

The variable called is never set back to false and therefore all the other methods are never executed, since called is set to true.

For example:

I opened the application, called at startup = false. The processing continues and called = true.

The application returned empty BUO data, since I did not click on any link when opening the application.

If I put the application in the background and click on a link, the application opens, but it does not return the data, since called = true and the response does not return to the application.

Yes, the MethodResultWrapper object is created for every new method call, ensuring that each call has its own called flag. This means that the called flag being set to true for one instance does not affect other instances created for subsequent method calls. Thus, each result is processed independently and does not interfere with others.

Similar implementation can be found here

@RodrigoSMarques
Copy link
Owner

Hi @RodrigoSMarques Thanks for the response
The changes are well-implemented and effectively prevent sending the result multiple times for the same method. The checkNotCalled method ensures that the flag is set to true the first time it is called, preventing subsequent calls from proceeding. This addresses the issue of sending the result multiple times.

Ok. But the result is strange.
The variable called is never set back to false and therefore all the other methods are never executed, since called is set to true.
For example:
I opened the application, called at startup = false. The processing continues and called = true.
The application returned empty BUO data, since I did not click on any link when opening the application.
If I put the application in the background and click on a link, the application opens, but it does not return the data, since called = true and the response does not return to the application.

Hi @RodrigoSMarques Thanks for the response
The changes are well-implemented and effectively prevent sending the result multiple times for the same method. The checkNotCalled method ensures that the flag is set to true the first time it is called, preventing subsequent calls from proceeding. This addresses the issue of sending the result multiple times.

Ok. But the result is strange.
The variable called is never set back to false and therefore all the other methods are never executed, since called is set to true.
For example:
I opened the application, called at startup = false. The processing continues and called = true.
The application returned empty BUO data, since I did not click on any link when opening the application.
If I put the application in the background and click on a link, the application opens, but it does not return the data, since called = true and the response does not return to the application.

Yes, the MethodResultWrapper object is created for every new method call, ensuring that each call has its own called flag. This means that the called flag being set to true for one instance does not affect other instances created for subsequent method calls. Thus, each result is processed independently and does not interfere with others.

Similar implementation can be found here

I recorded a video of the test run

@Junglee-Faisal
Copy link
Contributor Author

I recorded a video of the test run

You are testing this for the Eventchannel it won't work for the EventChannels.
The changes that I have done is for the MethodChannelWrapper class can you please check there?

@RodrigoSMarques
Copy link
Owner

I recorded a video of the test run

You are testing this for the Eventchannel it won't work for the EventChannels. The changes that I have done is for the MethodChannelWrapper class can you please check there?

You are correct.

Since the changes were simple, I wanted to replicate them in the code without merging.

It was too late, I changed the wrong file.

I will test again.

@Junglee-Faisal
Copy link
Contributor Author

You are correct.

Since the changes were simple, I wanted to replicate them in the code without merging.

It was too late, I changed the wrong file.

I will test again.

Sure, looking forward to your reply :)

@RodrigoSMarques RodrigoSMarques merged commit a419f17 into RodrigoSMarques:dev Jan 16, 2025
2 checks passed
@RodrigoSMarques
Copy link
Owner

@Junglee-Faisal

All right. I'll prepare the new release later.

Thanks

RodrigoSMarques added a commit that referenced this pull request Jan 17, 2025
### ⚠️ BREAKING CHANGE
* Minimum required Dart SDK version 3.3.0 (Flutter 3.19.0 - 15/02/2024)

### 🐛 Bug Fixes
* Fix issue #410: "reply already sent and a possible ANR". Tks @Junglee-Faisal

### 🎉 Features
* Migrated Gradle to declarative plugins block
@RodrigoSMarques RodrigoSMarques mentioned this pull request Jan 17, 2025
@RodrigoSMarques
Copy link
Owner

@Junglee-Faisal

Release 8.3.2 with fix, available.

@Junglee-Faisal
Copy link
Contributor Author

Thanks @RodrigoSMarques

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

Successfully merging this pull request may close these issues.

2 participants