Skip to content
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

Module exports can be called before their init function has completed #144

Open
reticulatedpines opened this issue Apr 25, 2024 · 0 comments

Comments

@reticulatedpines
Copy link
Owner

Spotted with dual_iso.

shoot.c called registered cbrs:

#if defined(CONFIG_MODULES)
        module_exec_cbr(CBR_SHOOT_TASK);
#endif

These are listed at the end of dual_iso.c:

MODULE_CBR(CBR_SHOOT_TASK, isoless_refresh, CTX_SHOOT_TASK)
MODULE_CBR(CBR_SHOOT_TASK, isoless_playback_fix, CTX_SHOOT_TASK)

Also in dual_iso.c, isoless_init() creates isoless_sem. But the shoot.c code can trigger before this, and isoless_refresh() did take_semaphore(isoless_sem, 0); with no guard for null pointer. On old cams, this is non-fatal, returning an error (which we didn't check for), but on new cams this triggers an OS assert, hanging the cam.

We might want to wrap take_semaphore() to do the null check before calling, it's an easy way to hang the cam.

It's highly unintuitive that the various module init functions don't run first. We might want module loading to have some globally visible manner to say when the init functions are finished, probably a semaphore that module_exec_cbr() would wait on. We should check for other routes that can call module exports, and have some appropriate guard there, too.

reticulatedpines added a commit that referenced this issue Apr 30, 2024
This creates the semaphore in the proper state,
and does required null checks to avoid issue:
#144
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

No branches or pull requests

1 participant