From: Zebediah Figura Subject: [PATCH] server: Use a free list for unallocated object handles. Message-Id: <20220126212418.10066-1-zfigura@codeweavers.com> Date: Wed, 26 Jan 2022 15:24:18 -0600 The Legend of Heroes: Trails of Cold Steel III suffers from an application bug, where it tries to wait on a handle after closing it. Because of the usage patterns of the game and the way Wine allocates handles, the handle is usually reused by a separate object, which is effectively never signaled, resulting in a hang. This patch changes our handle allocation strategy to resemble Windows, and in the process makes the race much less likely, although still theoretically possible. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52461 Signed-off-by: Zebediah Figura --- A more detailed explanation of the bug can be found in the linked bug report. server/handle.c | 125 +++++++++++++++++++++++++----------------------- 1 file changed, 65 insertions(+), 60 deletions(-) diff --git a/server/handle.c b/server/handle.c index bc692b8ebeb..75b1f7969df 100644 --- a/server/handle.c +++ b/server/handle.c @@ -41,8 +41,13 @@ struct handle_entry { - struct object *ptr; /* object */ - unsigned int access; /* access rights */ + union + { + struct object *ptr; /* object */ + struct handle_entry *next; /* next free entry */ + } u; + unsigned int access; /* access rights */ + int used; /* is the entry currently in use? */ }; struct handle_table @@ -51,8 +56,8 @@ struct handle_table struct process *process; /* process owning this table */ int count; /* number of allocated entries */ int last; /* last used entry */ - int free; /* first entry that may be free */ struct handle_entry *entries; /* handle entries */ + struct handle_entry *freelist; /* head of free list */ }; static struct handle_table *global_table; @@ -157,11 +162,11 @@ static void handle_table_dump( struct object *obj, int verbose ) entry = table->entries; for (i = 0; i <= table->last; i++, entry++) { - if (!entry->ptr) continue; + if (!entry->used) continue; fprintf( stderr, " %04x: %p %08x ", - index_to_handle(i), entry->ptr, entry->access ); - dump_object_name( entry->ptr ); - entry->ptr->ops->dump( entry->ptr, 0 ); + index_to_handle(i), entry->u.ptr, entry->access ); + dump_object_name( entry->u.ptr ); + entry->u.ptr->ops->dump( entry->u.ptr, 0 ); } } @@ -176,12 +181,14 @@ static void handle_table_destroy( struct object *obj ) for (i = 0, entry = table->entries; i <= table->last; i++, entry++) { - struct object *obj = entry->ptr; - entry->ptr = NULL; - if (obj) + if (entry->used) { + struct object *obj = entry->u.ptr; if (table->process) obj->ops->close_handle( obj, table->process, index_to_handle(i) ); + entry->used = 0; + entry->u.next = table->freelist; + table->freelist = entry; release_object_from_handle( obj ); } } @@ -208,7 +215,7 @@ struct handle_table *alloc_handle_table( struct process *process, int count ) table->process = process; table->count = count; table->last = -1; - table->free = 0; + table->freelist = NULL; if ((table->entries = mem_alloc( count * sizeof(*table->entries) ))) return table; release_object( table ); return NULL; @@ -234,21 +241,23 @@ static int grow_handle_table( struct handle_table *table ) /* allocate the first free entry in the handle table */ static obj_handle_t alloc_entry( struct handle_table *table, void *obj, unsigned int access ) { - struct handle_entry *entry = table->entries + table->free; - int i; + struct handle_entry *entry; - for (i = table->free; i <= table->last; i++, entry++) if (!entry->ptr) goto found; - if (i >= table->count) + if (table->freelist) { - if (!grow_handle_table( table )) return 0; - entry = table->entries + i; /* the entries may have moved */ + entry = table->freelist; + table->freelist = entry->u.next; } - table->last = i; - found: - table->free = i + 1; - entry->ptr = grab_object_for_handle( obj ); + else + { + if (table->last + 1 >= table->count && !grow_handle_table( table )) return 0; + entry = &table->entries[++table->last]; + } + + entry->used = 1; + entry->u.ptr = grab_object_for_handle( obj ); entry->access = access; - return index_to_handle(i); + return index_to_handle( entry - table->entries ); } /* allocate a handle for an object, incrementing its refcount */ @@ -327,31 +336,10 @@ static struct handle_entry *get_handle( struct process *process, obj_handle_t ha if (index < 0) return NULL; if (index > table->last) return NULL; entry = table->entries + index; - if (!entry->ptr) return NULL; + if (!entry->used) return NULL; return entry; } -/* attempt to shrink a table */ -static void shrink_handle_table( struct handle_table *table ) -{ - struct handle_entry *entry = table->entries + table->last; - struct handle_entry *new_entries; - int count = table->count; - - while (table->last >= 0) - { - if (entry->ptr) break; - table->last--; - entry--; - } - if (table->last >= count / 4) return; /* no need to shrink */ - if (count < MIN_HANDLE_ENTRIES * 2) return; /* too small to shrink */ - count /= 2; - if (!(new_entries = realloc( table->entries, count * sizeof(*new_entries) ))) return; - table->count = count; - table->entries = new_entries; -} - static void inherit_handle( struct process *parent, const obj_handle_t handle, struct handle_table *table ) { struct handle_entry *dst, *src; @@ -362,8 +350,9 @@ static void inherit_handle( struct process *parent, const obj_handle_t handle, s src = get_handle( parent, handle ); if (!src || !(src->access & RESERVED_INHERIT)) return; index = handle_to_index( handle ); - if (dst[index].ptr) return; - grab_object_for_handle( src->ptr ); + if (dst[index].used) return; + + grab_object_for_handle( src->u.ptr ); dst[index] = *src; table->last = max( table->last, index ); } @@ -397,6 +386,18 @@ struct handle_table *copy_handle_table( struct process *process, struct process { inherit_handle( parent, std_handles[i], table ); } + + /* iterate in reverse so that low values are allocated first */ + for (i = table->last; i >= 0; --i) + { + struct handle_entry *entry = &table->entries[i]; + + if (!entry->used) + { + entry->u.next = table->freelist; + table->freelist = entry; + } + } } else { @@ -406,14 +407,16 @@ struct handle_table *copy_handle_table( struct process *process, struct process memcpy( ptr, parent_table->entries, (table->last + 1) * sizeof(struct handle_entry) ); for (i = 0; i <= table->last; i++, ptr++) { - if (!ptr->ptr) continue; - if (ptr->access & RESERVED_INHERIT) grab_object_for_handle( ptr->ptr ); - else ptr->ptr = NULL; /* don't inherit this entry */ + if (ptr->used && (ptr->access & RESERVED_INHERIT)) grab_object_for_handle( ptr->u.ptr ); + else + { + ptr->used = 0; + ptr->u.next = table->freelist; + table->freelist = ptr; + } } } } - /* attempt to shrink the table */ - shrink_handle_table( table ); return table; } @@ -426,12 +429,14 @@ unsigned int close_handle( struct process *process, obj_handle_t handle ) if (!(entry = get_handle( process, handle ))) return STATUS_INVALID_HANDLE; if (entry->access & RESERVED_CLOSE_PROTECT) return STATUS_HANDLE_NOT_CLOSABLE; - obj = entry->ptr; + obj = entry->u.ptr; if (!obj->ops->close_handle( obj, process, handle )) return STATUS_HANDLE_NOT_CLOSABLE; - entry->ptr = NULL; + table = handle_is_global(handle) ? global_table : process->handles; - if (entry < table->entries + table->free) table->free = entry - table->entries; - if (entry == table->entries + table->last) shrink_handle_table( table ); + entry->used = 0; + entry->u.next = table->freelist; + table->freelist = entry; + release_object_from_handle( obj ); return STATUS_SUCCESS; } @@ -471,7 +476,7 @@ struct object *get_handle_obj( struct process *process, obj_handle_t handle, set_error( STATUS_INVALID_HANDLE ); return NULL; } - obj = entry->ptr; + obj = entry->u.ptr; if (ops && (obj->ops != ops)) { set_error( STATUS_OBJECT_TYPE_MISMATCH ); /* not the right type */ @@ -513,8 +518,8 @@ obj_handle_t find_inherited_handle( struct process *process, const struct object for (i = 0, ptr = table->entries; i <= table->last; i++, ptr++) { - if (!ptr->ptr) continue; - if (ptr->ptr->ops != ops) continue; + if (!ptr->used) continue; + if (ptr->u.ptr->ops != ops) continue; if (ptr->access & RESERVED_INHERIT) return index_to_handle(i); } return 0; @@ -825,7 +830,7 @@ static int enum_handles( struct process *process, void *user ) for (i = 0, entry = table->entries; i <= table->last; i++, entry++) { - if (!entry->ptr) continue; + if (!entry->used) continue; if (!info->handle) { info->count++; @@ -836,7 +841,7 @@ static int enum_handles( struct process *process, void *user ) handle->owner = process->id; handle->handle = index_to_handle(i); handle->access = entry->access & ~RESERVED_ALL; - handle->type = entry->ptr->ops->type->index; + handle->type = entry->u.ptr->ops->type->index; handle->attributes = 0; if (entry->access & RESERVED_INHERIT) handle->attributes |= OBJ_INHERIT; if (entry->access & RESERVED_CLOSE_PROTECT) handle->attributes |= OBJ_PROTECT_CLOSE; -- 2.34.1