Skip to content
This repository has been archived by the owner on Nov 21, 2020. It is now read-only.

To GIL or not to GIL #10

Closed
McSinyx opened this issue Jan 9, 2020 · 6 comments
Closed

To GIL or not to GIL #10

McSinyx opened this issue Jan 9, 2020 · 6 comments
Labels
bug Something isn't working help wanted Extra attention is needed
Milestone

Comments

@McSinyx
Copy link
Owner

McSinyx commented Jan 9, 2020

As described in Cython's documentation, the Python GIL may need some special care. Currently all C++ classes are declared with nogil C++ routines are randomly marked with nogil and with gil, and I am unsure whether it is not a good practice.

Edit: the acquire and release of GIL are definitely problematic, and so far all discovered deadlocks are with resource loading:

  1. Access to a buffer being cached, either via Buffer or free (Deadlock if Buffer called on the resource being cached async #73).
  2. Streaming a decoder defined in Python (deriving from BaseDecoder). Note that if it's loaded into a Buffer, the caching process is done on the main thread and thus no deadlock occurs.
  3. Loading resources using custom FileIO, either as a stream (decode) or as a buffer. This can either be a deadlock or a segfault.
@McSinyx McSinyx added the discussion Design issue label Jan 9, 2020
@McSinyx McSinyx added this to the 0.2 milestone Jan 9, 2020
@McSinyx McSinyx linked a pull request Feb 27, 2020 that will close this issue
@McSinyx McSinyx added bug Something isn't working help wanted Extra attention is needed labels Mar 16, 2020
@McSinyx
Copy link
Owner Author

McSinyx commented Mar 16, 2020

A minimal example to reproduce would be

from palace import *

use_fileio(lambda name: open(name, 'rb'))
with Device() as dev, Context(dev) as ctx, decode('audio file').play(12345, 6) as src:
    while src.playing: ctx.update()

@McSinyx
Copy link
Owner Author

McSinyx commented Apr 24, 2020

I'd be grateful if you two @Huy-Ngo and @9a24f0 can do a little bit of research on this and try to solve it. If none of us figure out a fix by the end of avril, we can declare this as a technical debt and precede to release v0.2 anyway.

Edit: it's the end of the month, this is dropped from the list of 0.2-blocking issues.

@McSinyx
Copy link
Owner Author

McSinyx commented May 13, 2020

Since 1.0 is supposed to be the stable version, this should be fixed then.

@McSinyx McSinyx added this to the 1.0 milestone May 13, 2020
@Huy-Ngo
Copy link
Collaborator

Huy-Ngo commented May 28, 2020

C++ routines are randomly marked with nogil and with gil

I'm a bit confused here. Which routines are marked with with gil? As I can see, none in alure.pxd is marked with with gil. Do you mean the ones in Cython's implementation?

@McSinyx
Copy link
Owner Author

McSinyx commented May 28, 2020

There are C++ functions defined in palace.pyx too (mostly for interfacing, e.g. Decoder, MessageHandler, FileIO).

@McSinyx
Copy link
Owner Author

McSinyx commented Nov 12, 2020

Migrated to SourceHut TODO ~cnx/palace#1.

@McSinyx McSinyx closed this as completed Nov 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants