Memory Mapping in MP3File.readV2Tag() leads to JVM crashes

beaTunes allows users to upload crash logs to a central database, the next time the app is started after a crash occurred.

I've found, that one of the most often occurring crashes is related to MP3File#readV2Tag(). Specifically, when reading data from the memory mapped file, I see crashes with the problematic frame v ~StubRoutines::jbyte_disjoint_arraycopy.

A sample stack trace looks like this:

v  ~StubRoutines::jbyte_disjoint_arraycopy
J 13388 C2 org.jaudiotagger.tag.id3.framebody.AbstractID3v2FrameBody.read(Ljava/nio/ByteBuffer;)V (195 bytes) @ 0x000000010c4dff73 [0x000000010c4df520+0xa53]
J 12277 C1 org.jaudiotagger.tag.id3.framebody.AbstractID3v2FrameBody.<init>(Ljava/nio/ByteBuffer;I)V (15 bytes) @ 0x000000010abcbae4 [0x000000010abcb5a0+0x544]
j  org.jaudiotagger.tag.id3.framebody.FrameBodyAPIC.<init>(Ljava/nio/ByteBuffer;I)V+3
j  sun.reflect.GeneratedConstructorAccessor362.newInstance([Ljava/lang/Object;)Ljava/lang/Object;+110
J 13387 C2 org.jaudiotagger.tag.id3.AbstractID3v2Frame.readBody(Ljava/lang/String;Ljava/nio/ByteBuffer;I)Lorg/jaudiotagger/tag/id3/framebody/AbstractID3v2FrameBody; (558 bytes) @ 0x000000010c4a5a74 [0x000000010c4a5300+0x774]
J 12481 C1 org.jaudiotagger.tag.id3.ID3v23Frame.read(Ljava/nio/ByteBuffer;)V (785 bytes) @ 0x000000010c5d52cc [0x000000010c5ce280+0x704c]
J 13341 C1 org.jaudiotagger.tag.id3.ID3v23Tag.readFrames(Ljava/nio/ByteBuffer;I)V (392 bytes) @ 0x000000010c6bf8ec [0x000000010c6bd700+0x21ec]
J 13427 C1 org.jaudiotagger.tag.id3.ID3v23Tag.read(Ljava/nio/ByteBuffer;)V (191 bytes) @ 0x000000010c71aa14 [0x000000010c718960+0x20b4]
J 13299 C1 org.jaudiotagger.tag.id3.ID3v23Tag.<init>(Ljava/nio/ByteBuffer;Ljava/lang/String;)V (45 bytes) @ 0x000000010a75eb2c [0x000000010a75e960+0x1cc]
J 13089 C1 org.jaudiotagger.audio.mp3.MP3File.readV2Tag(Ljava/io/File;II)V (314 bytes) @ 0x000000010c321c34 [0x000000010c31f8a0+0x2394]
J 13293 C1 org.jaudiotagger.audio.mp3.MP3File.<init>(Ljava/io/File;IZ)V (237 bytes) @ 0x000000010b231bfc [0x000000010b22fa40+0x21bc]

I don't see a good reason why we use a memory mapped file here. To the contrary. Mapping a file to memory in Java has serious flaws. You cannot unmap a file, unless you use that Cleaner logic (as it's currently done in the code). And that logic seems to be a really bad idea. The fact that I see quite a few crashes related to that piece of code also makes me think, that either the Cleaner logic is flawed or that file mapping in general is just not a good idea, because of JVM issues that I cannot pinpoint.

Why not just read the portion of the file we need into a ByteBuffer (we could use a direct one, to avoid heap memory problems) and avoid all these tedious mmap issues?

Just to illustrate my point, I have attached a bunch of user submitted crash logs related to this issue.

Comments (4)