pkujhd / goloader

load and run golang code at runtime.
Apache License 2.0
497 stars 58 forks source link

`getitab` alloc memory for `itab` by `persistentalloc`, and that couldnt be free #81

Closed fumeboy closed 1 year ago

fumeboy commented 1 year ago

https://github.com/golang/go/blob/96d16803c2aae5407e99c2a1db79bb51d9e1c8da/src/runtime/iface.go#L67

progToPointerMask (the function which generate gcmask) also usepersistentalloc

we cannot solve this memory leak case. I opened this issue just to remind that some memory will still remain in the runtime after the module is unloaded.

pkujhd commented 1 year ago

This memory cannot be released. remove itab from itabtable to avoid invalid address errors if it is not allocated to the same block of memory during load the same obj file again.

eh-steve commented 1 year ago

Also it's worth noting that this code path is only hit for relatively infrequent dynamic interface assignments (not static ones, where an itab would have been emitted by the compiler and so would be in mmapped memory instead)

fumeboy commented 1 year ago

https://github.com/golang/go/blob/3e7ec131667c31448365f47643cf9b58d08ffd26/src/runtime/heapdump.go#L118

another point worth paying attention to, runtime/debug.WriteHeapDump will store *_type to typecache

pkujhd commented 1 year ago

https://github.com/golang/go/blob/3e7ec131667c31448365f47643cf9b58d08ffd26/src/runtime/heapdump.go#L118

another point worth paying attention to, runtime/debug.WriteHeapDump will store *_type to typecache

@fumeboy This seems to be only used during call heapdump. *_type is stored in typecache, but it only use pointer for comparison. If the type does not exist, adding it. it seems does not cause problem, but it retains useless data. I tested dump heap when loaded the module, and dump heap after unload module, the process exits as normal

eh-steve commented 1 year ago

Yeah, types are only dumped which are currently in the itabtable, so old types that have been cached are never indirected (they should eventually get bumped off the bucket of 4 if enough hash collisions happen).

fumeboy commented 1 year ago

https://github.com/golang/go/blob/a031f4ef83edc132d5f49382bfef491161de2476/src/reflect/type.go#L1558

reflect's lookupCache will also keep pointer to _type, and it is safe to delete.