Thursday, July 28, 2011

Agit and zero-bytes the killer

As soon as a software developer is trying anything unusual, they stand a good chance of encountering our mighty friend – The Bug. When I Agitwrote, Git Androidclient, I stumbled upon a strange little bug that nearly stopped me dead in my tracks. Although I knew it at the time, the error was actually hidden deep in the source code for Android itself.

At guardian, we encountered a lot of interesting bugs (' Cos we make massaofinteresting thing) and the process of fixing them is usually made much easier because we have a tendency to use Open Source Software, though I have worked on Agit in my spare time, I got to play a lot of the same skills in pursuit of this error.

This is the second blog post about the development of Agit; You can read the first entry in this hereseries, which provides an overview of Agit and Git generally.

Android apps running on a version of the Java platform, and JGit, given a fantastic open source Java implementation of Git, which should have been any problem to get Git and runs on an Android phone ... yet mysterious, there was. using JGit to clone a list of test the Git repositories for ideas worked perfectly on my laptop (with the default version of Java is provided by the Sun, which was), but the work with only a few of them, when I ran it on my phone – mostly, it crashed, throw me this message: Pack file corruption detected: Unknown zlib error.

So ... Java on your portable computer, Java on your phone. What is the difference?

Android apps run on a Java-like set of libraries based on open-source Apache harmony project. Apache wrote this code from scratch – and file manager could get a license from Sun for the Java compatibility test kit — so it is not entirely the same as regular Sun Java. I suspected that somewhere between the two there was a difference is causing my problem – and I need to work, just where it was.

Identify the difference by running code on my phone would have been difficult, but fortunately I was able to dedicate to – harmony version of Java is easily installed on a laptop, and when I did was I could see the same behaviour again, with a code deficiency in harmony VM. There was great, because that confirmed that the problem was a mismatch between Sun vs. harmony Java, but it is not immediately tell me what the difference was. It is not also explain why some repos has consistently worked with harmony – what was the poison pill, which broke the other?

I added logging to the central parts of the code JGit. Error occurring when JGit "unpacking" Git pack file – pack files keep the lion's share of your repo data will be stored in a special compressed format. JGit user Inflater Java class to uncompress this stored data, and then do a bunch of checks to ensure that no data has been corrupted. Both harmony and Sun performs this identical, except for one anomaly: When JGit ran on harmony, and had the whole of an item is uncompressed, it sometimes would stop there it would keep on going, stumble when it was found that there were no more data to be had, and "Unknown zlib error" would be thrown.

Why would it stop? Code document lines around, waiting finished () flag Inflater to return "true" – but it never did. Responsibility to set the flag to true lay in the native code implementing the Inflater, never even getting called in this code, and this is why:
public int inflate (byte [] buf, int off, int nbytes) {
If (nbytes == 0) {
return 0;
}
//actually read compressed data and inflate it
...
}

As version of Inflater. inflate () had this small little optimization is not in the Sun version, as would short-cut the rest of the function if asked to inflate zero bytes of data – which sounds fine, right? Reading zero bytes should never change the mode of anything. Unfortunately, it just doesn't work so well If your repository happens to contain a zero-length file throughout the story, because this zero-length data stream will never get past the shortcut, and will never even be examined to determine that it is already finished – finished flag never gets set.

Zero length files are more common that you might think. Useful instructions displayed by Drupal support on the establishment of a new repository includes these commands:
Git init
Touch READ ME
Git add README
Git commit-m ' initial commit '

The command "touch" creates a zero-length file — and it's poison pill. All repositorie of poison pill died in harmony, and all the other fine.

Fixing the problem came in two parts – the narrow patch (simply useful to Agit) was to create a "harmonyfixinflater" subclass of Inflater as overrode inflate method (), then patching JGit use this Inflater instead ... but the broader patch fixing Android itself — a small change to the source code, but how'd I get weird little patch noticed and accepted?

Submitting patches is a delicate business. You should briefly explain where you come from, and not sound like a raving lunatic. So I raised the ticket against a harmony, and when that had been accepted, second a a against Android, refers to the first to give evidence, I am not talking rubbish, and provides a brief code snippet code to detect the problem.

This approach, methodical, as it was, missed a huge opportunity – I am not aware at the time, Android has a open web-based code-proofing system , everyone can contribute to. Code submitted to merge directly into tree, Android, makes it trivial for them to accept good patches. This is for me just jaw-droppingly amazing. Android is often called "open" because You can easily check the code out, but almost nothing is made of, they have a well-developed system for receiving changes back in – completely unlike any other commercial smartphone platform, which I am aware of.

Despite my ignorance of Androids code review system, the patch did get accepted, and made it onto Android devices with the release of Honeycomb – the latest Android tablets, Rails has it all, does not require the Agit. because the user "harmonyfixinflater" to work on pre-Honeycomb devices – but it is good to know that the problem is resolved for future developers.


View the original article here

No comments:

Post a Comment