Skip to content

Commit 4dc0a89

Browse files
committed
added proper cleanup for queued messages; removed some debugging stuff preparing for alpha release
1 parent 7221c17 commit 4dc0a89

File tree

1 file changed

+90
-32
lines changed

1 file changed

+90
-32
lines changed

module/new/binder.c

Lines changed: 90 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ static struct bcmd_msg *binder_realloc_msg(struct bcmd_msg *msg, size_t data_siz
517517
return binder_alloc_msg(data_size, offsets_size);
518518
}
519519

520-
static inline int binder_inform_obj(struct binder_obj *obj, unsigned int cmd)
520+
static inline int binder_write_cmd(msg_queue_id q, void *binder, void *cookie, unsigned int cmd)
521521
{
522522
struct bcmd_msg *msg;
523523
int r;
@@ -527,10 +527,10 @@ static inline int binder_inform_obj(struct binder_obj *obj, unsigned int cmd)
527527
return -ENOMEM;
528528

529529
msg->type = cmd;
530-
msg->binder = obj->binder;
531-
msg->cookie = obj->cookie;
530+
msg->binder = binder;
531+
msg->cookie = cookie;
532532

533-
r = bcmd_write_msg(obj->owner, msg);
533+
r = bcmd_write_msg(q, msg);
534534
if (r < 0) {
535535
kfree(msg);
536536
return r;
@@ -539,6 +539,11 @@ static inline int binder_inform_obj(struct binder_obj *obj, unsigned int cmd)
539539
return 0;
540540
}
541541

542+
static inline int binder_inform_owner(struct binder_obj *obj, unsigned int cmd)
543+
{
544+
return binder_write_cmd(obj->owner, obj->binder, obj->cookie, cmd);
545+
}
546+
542547
static int _binder_free_obj(struct binder_proc *proc, struct binder_obj *obj)
543548
{
544549
debugfs_remove(obj->info_node);
@@ -561,6 +566,7 @@ static int _binder_free_obj(struct binder_proc *proc, struct binder_obj *obj)
561566
msg->type = BR_DEAD_BINDER;
562567
msg->binder = obj->binder;
563568
msg->cookie = notifier->cookie;
569+
msg->reply_to = obj->owner; // identify the owner
564570
if (!bcmd_write_msg(notifier->to_notify, msg))
565571
msg = NULL;
566572

@@ -571,21 +577,68 @@ static int _binder_free_obj(struct binder_proc *proc, struct binder_obj *obj)
571577
} else {
572578
// reference - tell the owner we are no longer referencing the object
573579
if (atomic_read(&obj->refs) > 0)
574-
binder_inform_obj(obj, BC_RELEASE);
580+
binder_inform_owner(obj, BC_RELEASE);
575581
}
576582

577583
kfree(obj);
578584
return 0;
579585
}
580586

581-
static int binder_free_obj(struct binder_proc *proc, struct binder_obj *obj)
587+
static int binder_free_obj(struct binder_proc *proc, struct binder_obj *obj, int force)
582588
{
583589
spin_lock(&proc->obj_lock);
584590
rb_erase(&obj->rb_node, &proc->obj_tree);
585591
hlist_del(&obj->hash_node);
586592
spin_unlock(&proc->obj_lock);
587593

588-
return _binder_free_obj(proc, obj);
594+
if (!force)
595+
return _binder_free_obj(proc, obj);
596+
597+
debugfs_remove(obj->info_node);
598+
kfree(obj);
599+
return 0;
600+
}
601+
602+
static int drain_msg_buf(struct binder_proc *proc, struct bcmd_msg *msg)
603+
{
604+
struct bcmd_msg_buf *mbuf = msg->buf;
605+
606+
if (mbuf->offsets_size > 0) {
607+
struct flat_binder_object *bp;
608+
size_t *p, *ep, off;
609+
int n;
610+
611+
p = (size_t *)mbuf->offsets;
612+
ep = (size_t *)((char *)mbuf->offsets + mbuf->offsets_size);
613+
n = 0;
614+
615+
while (p < ep) {
616+
off = *p++;
617+
if (off + sizeof(*bp) > mbuf->data_size)
618+
return -EINVAL;
619+
620+
bp = (struct flat_binder_object *)(mbuf->data + off);
621+
switch (bp->type) {
622+
case BINDER_TYPE_BINDER:
623+
case BINDER_TYPE_HANDLE:
624+
if (mbuf->owners[n] != msg_queue_id(proc->queue))
625+
binder_write_cmd(mbuf->owners[n], bp->binder, bp->cookie, BC_RELEASE);
626+
break;
627+
628+
case BINDER_TYPE_FD:
629+
if (bp->binder)
630+
fput(bp->binder);
631+
break;
632+
633+
default:
634+
break;
635+
}
636+
637+
n++;
638+
}
639+
}
640+
641+
return 0;
589642
}
590643

591644
static void drain_msg_queue(struct binder_proc *proc, struct msg_queue *q)
@@ -596,11 +649,16 @@ static void drain_msg_queue(struct binder_proc *proc, struct msg_queue *q)
596649
while ((entry = msg_queue_pop(q))) {
597650
msg = container_of(entry, struct bcmd_msg, list);
598651

599-
//TODO: call free buffer
600-
if ((msg->type == BC_TRANSACTION) && !(msg->flags & TF_ONE_WAY)) {
601-
msg->type = BR_DEAD_REPLY;
602-
if (!bcmd_write_msg(msg->reply_to, msg))
603-
continue;
652+
if (msg->type == BC_TRANSACTION) {
653+
drain_msg_buf(proc, msg);
654+
655+
if (!(msg->flags & TF_ONE_WAY)) {
656+
msg->type = BR_DEAD_REPLY;
657+
if (!bcmd_write_msg(msg->reply_to, msg))
658+
continue;
659+
}
660+
} else if (msg->type == BC_REPLY) {
661+
drain_msg_buf(proc, msg);
604662
} else if (msg->type == BC_CLEAR_DEATH_NOTIFICATION) {
605663
struct binder_obj *obj;
606664

@@ -899,7 +957,7 @@ static inline int binder_release_obj(struct binder_proc *proc, struct binder_thr
899957

900958
// regardless it's an object or a reference, it should cease to exist now
901959
// TODO: review the risk of deleting the object here
902-
r = binder_free_obj(proc, obj);
960+
r = binder_free_obj(proc, obj, 0);
903961
if (r < 0)
904962
return r;
905963
}
@@ -942,7 +1000,7 @@ static int bcmd_write_flat_obj(struct binder_proc *proc, struct binder_thread *t
9421000
bp->cookie = obj->cookie;
9431001
*owner = obj->owner;
9441002

945-
r = binder_inform_obj(obj, BC_ACQUIRE);
1003+
r = binder_inform_owner(obj, BC_ACQUIRE);
9461004
if (r < 0)
9471005
return r;
9481006
break;
@@ -1001,7 +1059,7 @@ static int bcmd_read_flat_obj(struct binder_proc *proc, struct binder_thread *th
10011059
if (atomic_inc_return(&obj->refs) > 1) {
10021060
/* We aleady have a reference to the object, so tell
10031061
the owner to decrease one reference */
1004-
r = binder_inform_obj(obj, BC_RELEASE);
1062+
r = binder_inform_owner(obj, BC_RELEASE);
10051063
if (r < 0)
10061064
return r;
10071065
}
@@ -1019,7 +1077,7 @@ static int bcmd_read_flat_obj(struct binder_proc *proc, struct binder_thread *th
10191077
}
10201078

10211079
fd_install(fd, file);
1022-
bp->handle = fd; // TODO: fput() when free unread msg!!!
1080+
bp->handle = fd;
10231081
break;
10241082

10251083
default:
@@ -1069,7 +1127,7 @@ static int bcmd_write_msg_buf(struct binder_proc *proc, struct binder_thread *th
10691127
2. A -> B -> C -> A. In this case, B's transaction is delivered to C's process queue, but C's transaction
10701128
is delivered to thread A.
10711129
1072-
It doesn't impose above thread preference if a caller thread makes consective calls to more than one destination
1130+
It doesn't impose above thread preference if a caller thread makes consecutive calls to more than one destination
10731131
before getting the reply back. For example, like in case 2, if A -> B -> C, and before getting reply back from B,
10741132
A makes another call to C (A -> C), in which case, the second message is not guranteed to be delivered to thread
10751133
C who received B's transaction (triggered by A's transaction).
@@ -1650,17 +1708,23 @@ static long bcmd_read_transaction_complete(struct binder_proc *proc, struct bind
16501708

16511709
static long bcmd_read_dead_binder(struct binder_proc *proc, struct binder_thread *thread, struct bcmd_msg **pmsg, void __user *buf, unsigned long size)
16521710
{
1653-
uint32_t cmd = (*pmsg)->type, cookie = (uint32_t)(*pmsg)->cookie;
1711+
struct bcmd_msg *msg = *pmsg;
1712+
uint32_t cmd = msg->type, cookie = (uint32_t)msg->cookie;
1713+
struct binder_obj *obj;
16541714

16551715
if (size < sizeof(cmd) * 2)
16561716
return -ENOSPC;
16571717

1658-
if (put_user(cmd, (uint32_t *)buf) ||
1659-
put_user(cookie, (uint32_t *)((char *)buf + sizeof(cmd))))
1660-
return -EFAULT;
1718+
obj = binder_find_obj(proc, msg->reply_to, msg->binder);
1719+
if (obj) {
1720+
binder_free_obj(proc, obj, 1);
16611721

1662-
printk("pid %d/%d (tid %d/%d) read DEAD_BINDER command with cookie %08x\n", proc->pid, task_tgid_vnr(current), thread->pid, task_pid_vnr(current), cookie);
1663-
kfree(*pmsg);
1722+
if (put_user(cmd, (uint32_t *)buf) ||
1723+
put_user(cookie, (uint32_t *)((char *)buf + sizeof(cmd))))
1724+
return -EFAULT;
1725+
}
1726+
1727+
kfree(msg);
16641728
*pmsg = NULL;
16651729
return sizeof(cmd) * 2;
16661730
}
@@ -1680,7 +1744,6 @@ static long bcmd_read_dead_reply(struct binder_proc *proc, struct binder_thread
16801744
if (put_user(cmd, (uint32_t *)buf))
16811745
return -EFAULT;
16821746

1683-
printk("pid %d (tid %d) read DEAD_REPLY command\n", proc->pid, thread->pid);
16841747
kfree(*pmsg);
16851748
*pmsg = NULL;
16861749
return sizeof(cmd);
@@ -1715,13 +1778,12 @@ static long bcmd_read_acquire(struct binder_proc *proc, struct binder_thread *th
17151778
} else {
17161779
if (atomic_dec_return(&obj->refs) == 0) {
17171780
cmd = BR_RELEASE;
1718-
binder_free_obj(proc, obj);
1781+
binder_free_obj(proc, obj, 0);
17191782
}
17201783
}
17211784
}
17221785

17231786
if (cmd) {
1724-
printk("pid %d (tid %d) read %s binder command on object %p/%p\n", proc->pid, thread->pid, cmd==BR_ACQUIRE?"ACQUIRE":"RELEASE", msg->binder, msg->cookie);
17251787
ref_cmd.cmd = cmd;
17261788
ref_cmd.binder = msg->binder;
17271789
ref_cmd.cookie = msg->cookie;
@@ -1896,19 +1958,15 @@ static inline int cmd_write_read(struct binder_proc *proc, struct binder_thread
18961958

18971959
if (bwr->write_size > 0) {
18981960
r = binder_thread_write(proc, thread, (void __user *)bwr->write_buffer + bwr->write_consumed, bwr->write_size);
1899-
if (r < 0) {
1900-
printk("proc %d (tid %d) BWR write %d\n", proc->pid, thread->pid, r);
1961+
if (r < 0)
19011962
return r;
1902-
}
19031963
bwr->write_consumed += r;
19041964
}
19051965

19061966
if (bwr->read_size > 0) {
19071967
r = binder_thread_read(proc, thread, (void __user *)bwr->read_buffer + bwr->read_consumed, bwr->read_size);
1908-
if (r < 0) {
1909-
printk("proc %d (tid %d) BWR read %d\n", proc->pid, thread->pid, r);
1968+
if (r < 0)
19101969
return r;
1911-
}
19121970
bwr->read_consumed += r;
19131971
}
19141972

0 commit comments

Comments
 (0)