Skip to content

Conversation

@no1wudi
Copy link
Collaborator

@no1wudi no1wudi commented Dec 1, 2021

  • Exception will terminate the wasm app, it's not necessary since app should check the result of dynamic allocation, and they can do some cleanup or fallback operation on fail instead of 'crash' directly.
  • In acquire_wait_info the key of hasn_map_find can be NULL thus there are
    many sesenless error log

In acquire_wait_info the key of hasn_map_find can be NULL thus there are
many sesenless error log

if (!map || !key) {
LOG_ERROR("HashMap find elem failed: map or key is NULL.\n");
if (!map) {
Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is that we should modify acquire_wait_info in wasm_shared_memory.c but not here, e.g. change to:

static AtomicWaitInfo *
acquire_wait_info(void *address, bool create)
{
    AtomicWaitInfo *wait_info = NULL;
    bh_list_status ret;

    if (address)    => add check here
        wait_info = (AtomicWaitInfo *)bh_hash_map_find(wait_map, address);

    if (!create)
        return wait_info;

@xujuntwt95329, what's you opinion? Qi found that "In acquire_wait_info the key of hasn_map_find can be NULL thus there are many senseless error logs".

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I agree with @wenyongh, the caller should avoid passing NULL pointer to bh_hash_map_find

@wenyongh wenyongh merged commit c8fe100 into bytecodealliance:main Dec 3, 2021
vickiegpt pushed a commit to vickiegpt/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
Don't throw exception when module_malloc memory failed:
- Exception will terminate the wasm app, it's not necessary since app can
  check the result of dynamic allocation and do some cleanup or fallback
  operation on failure instead of 'crash' directly.
- In acquire_wait_info, call hasn_map_find only when the address isn't NULL,
  or there are many senseless error logs
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.

3 participants