Bugzilla

Returning to look at this now. This looks more complicated than I initially thought. Notes for myself and whoever

- Bug 1542674 was fairly extensive in terms of changes, following that (and maybe even prior) there are some threading safety concerns.
- The `GetDebugInfo` paths we're dealing with are often recursive -- an object may need to call `GetDebugInfo` on a member as part of figuring out its own debug info.
- We have a mix of sync methods and async methods to do this. Bug 1542674 appears to use `RequestDebugInfo` for async methods, and `GetDebugInfo` for the sync ones.
  - The async methods don't resolve to debug info, but rater use promises as a sync mechanism where it's only safe to access the mutable debug info arg once the promise resolves.
- There's a mix of assumptions about access to data throughout the objects involved. Some expect access only on certain threads, some uses locking, some use atoms. This means we need to handle data on a cases by case basis to ensure safety.
- There's many functions here that don't document or assert their expected threading model -- i.e. a lot of functions (and data) require usage to take place on a specific thread to ensure safety, but this is implicit and not enforced or documented.

I find these last two points make the code hard for me to reason about.

I am less confident in my original plan of holding locks over further access. I'm going to take a some time to assess if a broader rework can fix both this issue and help reduce and/or document the complexities here.

Returning to look at this now. This looks more complicated than I initially thought. Notes for myself and whoever

- Bug 1542674 was fairly extensive in terms of changes, following that (and maybe even prior) there are some threading safety concerns.
- The `GetDebugInfo` paths we're dealing with are often recursive -- an object may need to call `GetDebugInfo` on a member as part of figuring out its own debug info.
- We have a mix of sync methods and async methods to do this. Bug 1542674 appears to use `RequestDebugInfo` for async methods, and `GetDebugInfo` for the sync ones.
  - The async methods don't resolve to debug info, but rather use promises as a sync mechanism where it's only safe to access the mutable debug info arg once the promise resolves.
- There's a mix of assumptions about access to data throughout the objects involved. Some expect access only on certain threads, some uses locking, some use atomics. This means we need to handle data on a cases by case basis to ensure safety.
- There's many functions here that don't document or assert their expected threading model -- i.e. a lot of functions (and data) require usage to take place on a specific thread to ensure safety, but this is implicit and not enforced or documented.

I find these last two points make the code hard for me to reason about.

I am less confident in my original plan of holding locks over further access. I'm going to take a some time to assess if a broader rework can fix both this issue and help reduce and/or document the complexities here.

Returning to look at this now. This looks more complicated than I initially thought. Notes for myself and whoever

- Bug 1542674 was fairly extensive in terms of changes, following that (and maybe even prior) there are some threading safety concerns.
- The `GetDebugInfo` paths we're dealing with are often recursive -- an object may need to call `GetDebugInfo` on a member as part of figuring out its own debug info.
- We have a mix of sync methods and async methods to do this. Bug 1542674 appears to use `RequestDebugInfo` for async methods, and `GetDebugInfo` for the sync ones.
  - The async methods don't resolve to debug info, but rather use promises as a sync mechanism where it's only safe to access the mutable debug info arg once the promise resolves.
- There's a mix of assumptions about access to data throughout the objects involved. Some expect access only on certain threads, some use locking, some use atomics. This means we need to handle data on a cases by case basis to ensure safety.
- There's many functions here that don't document or assert their expected threading model -- i.e. a lot of functions (and data) require usage to take place on a specific thread to ensure safety, but this is implicit and not enforced or documented.

I find these last two points make the code hard for me to reason about.

I am less confident in my original plan of holding locks over further access. I'm going to take a some time to assess if a broader rework can fix both this issue and help reduce and/or document the complexities here.