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

raising exception in __del__ finaliser results in deadlock with multithread enabled #3627

Open
adritium opened this issue Feb 21, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@adritium
Copy link

commented Feb 21, 2018

Original question is on uPy forum

call gc_alloc()
  GC_ENTER()
  ...
  GC_EXIT()
  call gc_collect()
    call gc_collect_start()
      GC_ENTER()
      call gc_collect_end()
        call gc_sweep()
          call "__del__" method of object if it exists (in this example an old file pointer)
            file_close() throws a "file not open" OS Exception
              OS Exception calls gc_alloc()
                gc_alloc() calls GC_ENTER() 
Result: The thread waits for the mutex to be released by itself.

The naming of m_malloc_maybe indicates it's supposed to be a conditional i.e. non-blocking allocation attempt at allocation but its implementation does not actually provide that feature; it just calls malloc and does a debug print.

void *m_malloc_maybe(size_t num_bytes) {
    void *ptr = malloc(num_bytes);
#if MICROPY_MEM_STATS
    MP_STATE_MEM(total_bytes_allocated) += num_bytes;
    MP_STATE_MEM(current_bytes_allocated) += num_bytes;
    UPDATE_PEAK();
#endif
    DEBUG_printf("malloc %d : %p\n", num_bytes, ptr);
    return ptr;
}
@stinos

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2018

The naming of m_malloc_maybe indicates it's supposed to be a conditional i.e. non-blocking allocation attempt at allocation but its implementation does not actually provide that feature; it just calls mallocand does a debug print.

See m_malloc right above it: m_malloc_baybe just sliently returns NULL if it cannot allocate, whereas m_malloc will call m_malloc_fail if it fails to allocate.

@adritium

This comment has been minimized.

Copy link
Author

commented Feb 21, 2018

I see that but it doesn't fix the deadlock in multi-threaded configuration.

malloc()->gc_alloc()->GC_ENTER()->mp_thread_mutex_lock(&MP_STATE_MEM(gc_mutex), 1)

@adritium

This comment has been minimized.

Copy link
Author

commented Feb 21, 2018

The problem is GC_ENTER() is hardcoded to call with wait = 1

#define GC_ENTER() mp_thread_mutex_lock(&MP_STATE_MEM(gc_mutex), 1)
#define GC_EXIT() mp_thread_mutex_unlock(&MP_STATE_MEM(gc_mutex))

int mp_thread_mutex_lock(mp_thread_mutex_t *mutex, int wait)

You'd have to redefine it
#define GC_ENTER(wait) if(mp_thread_mutex_lock(&MP_STATE_MEM(gc_mutex), wait) == NULL) return NULL;

and redefine void *gc_alloc(size_t n_bytes, bool has_finaliser) to add another argument to pass to GC_ENTER(wait)

@dpgeorge

This comment has been minimized.

Copy link
Member

commented Feb 23, 2018

Thanks for raising this from the forum. I replied on the forum about how to work around this issue, but it does look like it's a real bug that needs fixing. The first thing to do would be to reproduce it on the unix port. And the solution is to make sure the GC doesn't re-enter itself from the same thread. That is already protected against by the gc_lock_depth variable, but in a multithreading environment with the GIL disabled it doesn't work quite like it's supposed to.

@dpgeorge dpgeorge added the bug label Feb 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.