Skip to content

Commit

Permalink
Hold MessageListAdapters and MessageListViews in DeckAdapter to avoid…
Browse files Browse the repository at this point in the history 
… leaks

There are at least two significant memory leaks in Yaaic, which cause
the client to force close after a few hours with an
OutOfMemoryException:

(1) The IRCService holds Conversation objects, which contain a
MessageListAdapter, which have references to the ConversationActivity
context.  This causes Activity contexts to outlast the Activity, causing
a significant memory leak over time.

Fix this by holding the MessageListAdapter in the ConversationActivity's
DeckAdapter instead of in the Conversation objects.  The DeckAdapter's
lifecycle matches that of the Activity, so this prevents the leak.

(2) Every call to DeckAdapter.getView()/renderConversation() creates a
new MessageListView and adds it to the deck.  But adding the view to
the deck causes the deck to take a reference to the view, leaking the
MessageListView until the Activity is finished.  (This has the effect of
exacerbating the first leak, since the Activity context holds a
reference to the deck.)

Fix this leak by caching MessageListViews in the DeckAdapter, and
returning an existing MessageListView for a Conversation in getView() if
one already exists.
  • Loading branch information
@steven676 @pocmo
steven676 authored and pocmo committed Jun 8, 2011
1 parent c4504be commit ffe73b7
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 44 deletions.
11 changes: 7 additions & 4 deletions application/src/org/yaaic/activity/ConversationActivity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.yaaic.receiver.ConversationReceiver;
import org.yaaic.receiver.ServerReceiver;
import org.yaaic.view.ConversationSwitcher;
import org.yaaic.view.MessageListView;

import android.app.Activity;
import android.app.AlertDialog;
Expand Down Expand Up @@ -235,7 +236,7 @@ public void onResume()

// Fill view with messages that have been buffered while paused
for (Conversation conversation : mConversations) {
mAdapter = conversation.getMessageListAdapter();
mAdapter = deckAdapter.getItemAdapter(conversation.getName());

if (mAdapter != null) {
mAdapter.addBulkMessages(conversation.getBuffer());
Expand Down Expand Up @@ -401,7 +402,7 @@ public void onConversationMessage(String target)
return;
}

MessageListAdapter adapter = conversation.getMessageListAdapter();
MessageListAdapter adapter = deckAdapter.getItemAdapter(target);

while(conversation.hasBufferedMessages()) {
Message message = conversation.pollBufferedMessage();
Expand Down Expand Up @@ -526,8 +527,10 @@ public boolean onKeyDown(int keyCode, KeyEvent event)
{
if (keyCode == KeyEvent.KEYCODE_BACK && event.getRepeatCount() == 0) {
if (deckAdapter.isSwitched()) {
switcher.showNext();
switcher.removeView(deckAdapter.getSwitchedView());
MessageListView canvas = (MessageListView) deckAdapter.getView(deckAdapter.getPositionByName(deckAdapter.getSwitchedName()), null, switcher);
canvas.setLayoutParams(new Gallery.LayoutParams(switcher.getWidth()*85/100, switcher.getHeight()));
canvas.setTranscriptMode(MessageListView.TRANSCRIPT_MODE_ALWAYS_SCROLL);
canvas.setDelegateTouchEvents(true);
deckAdapter.setSwitched(null, null);
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion application/src/org/yaaic/activity/ServersActivity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,4 +344,4 @@ public void onStatusUpdate()
}
}

}
}
84 changes: 68 additions & 16 deletions application/src/org/yaaic/adapter/DeckAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,24 +41,36 @@
*/
public class DeckAdapter extends BaseAdapter
{
private LinkedList<Conversation> conversations;
private LinkedList<ConversationInfo> conversations;
private MessageListView currentView;
private String currentChannel;

public class ConversationInfo {
public Conversation conv;
public MessageListAdapter adapter;
public MessageListView view;

public ConversationInfo(Conversation conv) {
this.conv = conv;
this.adapter = null;
this.view = null;
}
}

/**
* Create a new DeckAdapter instance
*/
public DeckAdapter()
{
conversations = new LinkedList<Conversation>();
conversations = new LinkedList<ConversationInfo>();
}

/**
* Clear conversations
*/
public void clearConversations()
{
conversations = new LinkedList<Conversation>();
conversations = new LinkedList<ConversationInfo>();
}

/**
Expand All @@ -70,16 +82,51 @@ public int getCount()
return conversations.size();
}

/**
* Get ConversationInfo on item at position
*/
private ConversationInfo getItemInfo(int position) {
if (position >= 0 && position < conversations.size()) {
return conversations.get(position);
}
return null;
}

/**
* Get item at position
*/
@Override
public Conversation getItem(int position)
{
if (position >= 0 && position < conversations.size()) {
return conversations.get(position);
ConversationInfo convInfo = getItemInfo(position);
if (convInfo != null) {
return convInfo.conv;
} else {
return null;
}
return null;
}

/**
* Get MessageListAdapter belonging to a conversation
*
* @param position Position of the conversation in the deck
*/
public MessageListAdapter getItemAdapter(int position) {
ConversationInfo convInfo = getItemInfo(position);
if (convInfo != null) {
return convInfo.adapter;
} else {
return null;
}
}

/**
* Get MessageListAdapter belonging to a conversation
*
* @param name Name of the conversation
*/
public MessageListAdapter getItemAdapter(String name) {
return getItemAdapter(getPositionByName(name));
}

/**
Expand All @@ -99,7 +146,7 @@ public long getItemId(int position)
*/
public void addItem(Conversation conversation)
{
conversations.add(conversation);
conversations.add(new ConversationInfo(conversation));

notifyDataSetChanged();
}
Expand All @@ -114,10 +161,10 @@ public int getPositionByName(String name)
{
// Optimization - cache field lookups
int mSize = conversations.size();
LinkedList<Conversation> mItems = this.conversations;
LinkedList<ConversationInfo> mItems = this.conversations;

for (int i = 0; i < mSize; i++) {
if (mItems.get(i).getName().equalsIgnoreCase(name)) {
if (mItems.get(i).conv.getName().equalsIgnoreCase(name)) {
return i;
}
}
Expand Down Expand Up @@ -187,17 +234,21 @@ public boolean isSwitched()
@Override
public View getView(int position, View convertView, ViewGroup parent)
{
Conversation conversation = getItem(position);
ConversationInfo convInfo = getItemInfo(position);

// Market stack traces prove that sometimes we get a null converstion
// because the collection changed while a view is requested for an
// item that does not exist anymore... so we just need to reply with
// some kind of view here.
if (conversation == null) {
if (convInfo == null || convInfo.conv == null) {
return new TextView(parent.getContext());
}

return renderConversation(conversation, parent);
if (convInfo.view != null) {
return convInfo.view;
} else {
return renderConversation(convInfo, parent);
}
}

/**
Expand All @@ -207,16 +258,17 @@ public View getView(int position, View convertView, ViewGroup parent)
* @param parent The parent view (context)
* @return The rendered MessageListView
*/
public MessageListView renderConversation(Conversation conversation, ViewGroup parent)
private MessageListView renderConversation(ConversationInfo convInfo, ViewGroup parent)
{
MessageListView list = new MessageListView(parent.getContext(), parent);
convInfo.view = list;
list.setOnItemClickListener(MessageClickListener.getInstance());

MessageListAdapter adapter = conversation.getMessageListAdapter();
MessageListAdapter adapter = convInfo.adapter;

if (adapter == null) {
adapter = new MessageListAdapter(conversation, parent.getContext());
conversation.setMessageListAdapter(adapter);
adapter = new MessageListAdapter(convInfo.conv, parent.getContext());
convInfo.adapter = adapter;
}

list.setAdapter(adapter);
Expand Down
6 changes: 2 additions & 4 deletions application/src/org/yaaic/listener/ConversationClickListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import android.widget.AdapterView;
import android.widget.AdapterView.OnItemClickListener;
import android.widget.ListView;
import android.widget.TableLayout.LayoutParams;
import android.widget.Gallery.LayoutParams;
import android.widget.ViewSwitcher;

/**
Expand Down Expand Up @@ -61,13 +61,11 @@ public void onItemClick(AdapterView<?> adapterView, View view, int position, lon
{
Conversation conversation = adapter.getItem(position);

MessageListView canvas = adapter.renderConversation(conversation, switcher);
MessageListView canvas = (MessageListView) adapter.getView(position, null, switcher);
canvas.setTranscriptMode(ListView.TRANSCRIPT_MODE_NORMAL);
canvas.setLayoutParams(new LayoutParams(LayoutParams.FILL_PARENT, LayoutParams.FILL_PARENT));
canvas.setDelegateTouchEvents(false); // Do not delegate

adapter.setSwitched(conversation.getName(), canvas);
switcher.addView(canvas);
switcher.showNext();
}
}
19 changes: 0 additions & 19 deletions application/src/org/yaaic/model/Conversation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@

import java.util.LinkedList;

import org.yaaic.adapter.MessageListAdapter;

/**
* Base class for conversations
*
Expand All @@ -48,7 +46,6 @@ public abstract class Conversation
private final LinkedList<Message> buffer;
private final LinkedList<Message> history;
private final String name;
private MessageListAdapter adapter;
private int status = 1;

/**
Expand Down Expand Up @@ -148,22 +145,6 @@ public void clearBuffer()
buffer.clear();
}

/**
* Store the adapter of this conversation
*/
public void setMessageListAdapter(MessageListAdapter adapter)
{
this.adapter = adapter;
}

/**
* Get the MessageList Adapter of this conversation if known
*/
public MessageListAdapter getMessageListAdapter()
{
return adapter;
}

/**
* Set status of conversation
*
Expand Down

0 comments on commit ffe73b7

Please sign in to comment.