Mir

Code review comment for lp:~alan-griffiths/mir/bfi-fix-1416482

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

We need to serialize one level higher, in GLProgramFamily::add_program(), to ensure glCreateProgram() is also protected from concurrent access.

Note that we now have two points at which we allocate program and shader ids: GLProgramFamily
and GLProgramFactory (used only in the Android backend after a recent refactoring). Although they may be (or rather will be after this fix) invidually protected from concurrent access, in theory two threads could run concurrently in the two different classes. Not sure if it's a problem in practice, but ideally the two classes should also get synchronized or merged (not in this MP); we don't really need two implementations offering the same functionality.

Some background:

This was first addressed in r923 [1] :

/*
 * We need to serialize renderer creation because some GL calls used
 * during renderer construction that create unique resource ids
 * (e.g. glCreateProgram) are not thread-safe when the threads are
 * have the same or shared EGL contexts.
 */

Since then the code has been refactored multiple times and the serialization logic moved around. In a recent refactoring (the transition from GLProgramFactory to GLProgramFamily for GLRenderer), the serialization logic was lost.

[1] https://bazaar.launchpad.net/~mir-team/mir/0.11/revision/923

review: Needs Fixing

« Back to merge proposal