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.

Sign up for GitHub

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

mbox: provide function to unset initialized mbox #15485

Merged
merged 2 commits into from
Mar 20, 2024
Merged

mbox: provide function to unset initialized mbox #15485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
12194ad
mbox: provide function to unset initialized mbox
miri64 Nov 20, 2020
6229fda
lwip: use new mbox_unset() function
miri64 Nov 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions core/include/mbox.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,17 @@ static inline size_t mbox_avail(mbox_t *mbox)
return cib_avail(&mbox->cib);
}

/**
* @brief Unset's the mbox, effectively deinitializing and invalidating it.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From #15484 (comment)

This leaks a possibly supplied queue. Maybe better call it mbox_release_queue?
The mbox actually works without queue, it'd just be synchronous from that point on.

This is also a bit dangerous, if this is called with messages in the queue, they get lost. If there's a sender that was previously waiting, it would be stuck there until the queue transitions from full to empty again.

The sys_mbox_set_invalid() function is called whenever a netconn (lwIP's sock equivalent) is closed or finished, so this isn't a risk, but maybe that function should be fixed in the way that readers and writers are also released and removed somehow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I still see issues:

  • as is, this is not thread safe, something could send/rx a message between those two lines. I think mbox breaks if the array is NULL but cib mask not. (this could be mitigated by changing the order, but that seems hacky).
  • any waiters on that mbox would be blocked indefinitely.
    I don't see any nice way around this. even while (mbox_try_ receive(tmp) {} would be racey.

I'm fine adding this function (if needed), but maybe those should be fixed or mentioned?

*
* @param[in] mbox ptr to mailbox to operate on
*/
static inline void mbox_unset(mbox_t *mbox)
{
mbox->msg_array = NULL;
mbox->cib.mask = 0;
}

#ifdef __cplusplus
}
#endif
Expand Down
4 changes: 2 additions & 2 deletions pkg/lwip/include/arch/sys_arch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,13 @@ typedef struct {

static inline bool sys_mbox_valid(sys_mbox_t *mbox)
{
return (mbox != NULL) && (mbox->mbox.cib.mask != 0);
return (mbox != NULL) && (mbox_size(&mbox->mbox) != 0);
}

static inline void sys_mbox_set_invalid(sys_mbox_t *mbox)
{
if (mbox != NULL) {
mbox->mbox.cib.mask = 0;
mbox_unset(&mbox->mbox);
}
}

Expand Down
Loading