Skip to content

[w32process] clear lazy init class pointers when system image changes #1168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

corngood
Copy link

@corngood corngood commented Apr 3, 2019

Running a debug build of unity/mono, and opening a project which has dirty scripts results in the following:

  • stash_system_image is called
  • get_process_module_class is called, which lazy inits process_module_class to the loaded ProcessModule
  • on script reload, unity destroys the app domain, including the class object pointed to by process_module_class
  • stash_system_image is called again
  • get_process_module_class is called again, which returns a pointer to the dead object, resulting in a crash

This is the fix I'm currently testing. I haven't yet investigated the threading/atomic requirements of doing this.

Also, there are other similar patterns (e.g. https://github.com/Unity-Technologies/mono/blob/unity-master/mono/metadata/threadpool.c#L168) which look like they would suffer from the same problem. I'm not sure how these would work.

Anyone know how these lazy inits are supposed to survive app domain destruction?

The stack trace where the particular class object is destroyed is:

>	mono-2.0-bdwgc.dll!memset() Line 133	Unknown
 	mono-2.0-bdwgc.dll!free_dbg_nolock(void * const block, const int block_use) Line 985	C++
 	mono-2.0-bdwgc.dll!_free_dbg(void * block, int block_use) Line 1011	C++
 	mono-2.0-bdwgc.dll!free(void * block) Line 20	C++
 	mono-2.0-bdwgc.dll!monoeg_g_free(void * ptr) Line 67	C
 	mono-2.0-bdwgc.dll!mono_mempool_destroy(_MonoMemPool * pool) Line 139	C
 	mono-2.0-bdwgc.dll!mono_image_close_finish(_MonoImage * image) Line 2193	C
 	mono-2.0-bdwgc.dll!mono_assembly_close_finish(_MonoAssembly * assembly) Line 3889	C
 	mono-2.0-bdwgc.dll!mono_image_close_finish(_MonoImage * image) Line 2174	C
 	mono-2.0-bdwgc.dll!mono_assembly_close_finish(_MonoAssembly * assembly) Line 3889	C
 	mono-2.0-bdwgc.dll!mono_domain_free(_MonoDomain * domain, int force) Line 1158	C
 	mono-2.0-bdwgc.dll!unload_thread_main(void * arg) Line 2625	C

Josh Peterson and others added 17 commits June 5, 2019 14:50
This change builds the class library code to implement the
`NamedPipeClientStream` class in the unityaot profile on Windows. For
the time being, we will not implement this class for non-Windows
platforms, since that requires the Mono.Posix.dll assembly and a native
library as well.

We may consider adding support on Posix platforms in the future.
On macOS and FAT32 partitions, we will sometimes get this inode value
for more than one file. It means the file is empty.  When this happens,
the hash table of file shares becomes corrupt, since more then one file
has the same inode. Instead, let's assume it is always fine to share
empty files. (Unity case 950616).
Previously, 5174f7e was an attempt to
correct this issue. However, the `#define` scheme for Mono changed since
then, so `PLATFORM_MACOSX` became `HOST_DARWIN`.

This change uses the proper define to correct Unity case 1166108.
@joncham
Copy link
Member

joncham commented Oct 31, 2019

@corngood I ended up fixing this by just removing the caching as it's not really needed. This could never really work with appdomains as is.

Unity: #1237

Upstream PR: mono#17602

@joncham joncham closed this Oct 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants