-
Notifications
You must be signed in to change notification settings - Fork 1.1k
read logcat logs in nonblocking mode with end timeout #466
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
Conversation
What version of ACRA was this for? |
I reproduce it with version 4.8.5 / 4.9.0 rc 2; not sure about 4.6.3, a user reported similar behaviour but I didn't investigated than |
Patch was tested with 4.9.0 rc 2 |
String line; | ||
final List<String> buffer = limit == NO_LIMIT ? new LinkedList<String>() : new BoundedLinkedList<String>(limit); | ||
long end = System.currentTimeMillis() + READ_TIMEOUT; | ||
while ((System.currentTimeMillis() < end)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this mean that it will always block for 3 seconds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I've added a read timeout limit. Not sure if this should be exposed to ACRAConfiguration.
In my tests if timeout was reached in few seconds I received on remote server logs with empty logcat field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And if a developer don't want any delay on main / exception thread, acra's work should be done in a separate thread. For the moment I use a helper method in which I use a Runnable and start a new thread; again, not sure if a queue should be use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point is, is that it's not a timeout.
This read will block for exactly 3 seconds regardless of whether there is any log to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point. this is the fix
if ((line = nonBlockingReader.readLine()) != null) { if (filter.apply(line)) { buffer.add(line); } } else { break; }
NB when you submit a pull request, always submit it from a branch so that you don't risk polluting it with any other changes you might make. |
This means that I should do something next? |
If this runs into timeout, will the newest or the oldest logs miss? BTW: I wouldn't name this method Async, because this normally means that it returns asynchronous. Something more descriptive like NonBlocking would fit better. I also think this might not be a good idea as default, maybe an opt-in option for it would be better. |
If runs in timeout it will
|
I just checked out your master for testing purpose. It does not compile. I think you forgot to commit the addition of local attributes in ACRAConfiguration and ConfigurationBuilder. |
Fixed |
I encounter and issue with handleSilentException and ReportField.LOGCAT on Lenovo TAB 2 A8-50F.
It appears that when an ProgressDialog is shown a lot of useless info are spitted to the logs and ACRA spends couple of minutes to receive logs and send report due the fact that reading from InputStream is done in a blocking way.
I think is better to have at least an empty log info than a lots of useless logs due the fact that Stream is blocked for writing.
Also for my particular context handling ACRA's sending exception in another thread helped me, and received some info due the fact ProgressDialog is closed immediately after.
new Thread(new Runnable() { @Override public void run() { ACRA.getErrorReporter().handleSilentException(e); } }).start();
the_culprit.txt