Rasterio: Env() thread and process safety

[v0lat1le]

Using python 3.5 and rasterio 1.0a7 from conda-forge.

def func(i):
    with rasterio.Env() as env:
        print(i)
        return i

pool = ThreadPoolExecutor(8)
while True:
   r = list(pool.map(func, range(8)))

Eventually fails with: EnvError: No GDAL environment exists

<ipython-input-34-f30f473c00f3> in func(i)
      2     with rasterio.Env() as env:
      3         print(i)
----> 4         return i
      5 
      6 pool = ThreadPoolExecutor(8)

/g/data/u46/users/gxr547/conda/envs/agdc/lib/python3.5/site-packages/rasterio/env.py in __exit__(self, exc_type, exc_val, exc_tb)
    203         global _discovered_options
    204         log.debug("Exiting env context: %r", self)
--> 205         delenv()
    206         if self._has_parent_env:
    207             defenv()

/g/data/u46/users/gxr547/conda/envs/agdc/lib/python3.5/site-packages/rasterio/env.py in delenv()
    258     global _env
    259     if not _env:
--> 260         raise EnvError("No GDAL environment exists")
    261     else:
    262         _env.clear_config_options()

EnvError: No GDAL environment exists

I discovered that while using rasterio.open/read with dask arrays

[v0lat1le]

The only way, that I can think of, for Env to keep the same interface and behave safely is to use thread-local _env rather than global.

[geowurster]

@v0lat1le I hit this exact issue the other day as well and haven't had time to work out the correct solution, but I think this boils down to a documentation issue. I have been picking at some environment improvements on the env2 branch branch, some of which may help this situation as well.

A coworker is seeing behavior suggesting otherwise, but I _think_ if you instantiate a rasterio.Env() outside your threaded activity you should be good. Do you need to manipulate the environment inside the thread? That could get messy.

with rasterio.Env():
    pool = ThreadPoolExecutor(8)
    while True:
       r = list(pool.map(func, range(8)))

More specifically, I think the rasterio.Env() needs to wrap the call that triggers the thread pool to actually do work:

results = ThreadPoolExecutor(8).map(lambda x: x ** 2, range(10))
with rasterio.Env():
    for res in results:
        pass

[benjimin]

Time for an API redesign? (Pass the updated config around explicitly. Either as an optional keyword argument, or otherwise by implementing top-level rasterio functions as methods of an Env class, and ensuring the default instance is immutable.)

It seems reasonable that multiple parallel threads might specialise their environments differently, and also that the parent thread might leave the context manager before the (inheriting) children start working.

[v0lat1le]

@geowurster I'm not really manipulating the Env at all. My code looks more like:

def func(i):
    with rasterio.open(...) as src:
         ...

and with Env() as env: happens inside rasterio.open

It does look like env2 branch would resolve my specific issue since rasterio.env._ENV global is not mutated inside rasterio.open. There's still a race condition if different threads try to use different environments.

This

with rasterio.Env():
    pool = ThreadPoolExecutor(8)
    while True:
       r = list(pool.map(func, range(8)))

doesn't raise the exception, and would actually work in my case since I'm not changing the environment, but it just masks the real issue that Env.__enter__ and Env.__exit__ mutate unprotected global variable _env and that leads to trouble if multiple threads are involved

[geowurster]

GDAL only has a single environment so Rasterio must as well. Sticking the current Rasterio environment in rasterio.env._env is just how we keep track of which one is active. Attached to each environment instance are the parent instance's config option settings, which allows nested rasterio.Env() objects to teardown and reset the environment to the state it was in when the environment started. Changing the environment's state within a thread impacts all other threads. I'm not sure if its possible to create multiple isolated GDAL environments.

This has to do with who is responsible for tearing down the GDAL environment when. @v0lat1le I notice you wrapped your tests in while True so it runs until it fails, because it doesn't _always_ fail. This should fail every time and is hopefully a clearer illustration of what's happening:

from threading import Thread
import time

import rasterio as rio


def func(sleeper):
    with rio.open('tests/data/RGB.byte.tif') as src:
        time.sleep(sleeper)


t1 = Thread(target=func, args=(0.5,))
t2 = Thread(target=func, args=(0,))

t1.start()
t2.start()

t1.join()
t2.join()

What's happening is that the rasterio.open() in the first thread sees that an environment does not already exist so it creates one. Thread 2 exits before thread 1, and because of a bug in the current version of Rasterio it tears down the environment, so when Thread 1 exits it throws an error because it can't find a GDAL environment when it thinks it should. Wrapping the start() and join() calls in a single rasterio.Env() causes both threads to see this existing environment and not create one.

Simplifying environment teardown is most of what I fixed in env2, and if you run the same code snippet above on env2 it succeeds without wrapping everything in rasterio.Env() because the second thread completely inherits the environment the first thread created. This may seem a bit counterintuitive but if you don't need to modify the config options at all it doesn't matter, and if you do you can just wrap everything in rasterio.Env() so each thread grabs your explicitly defined environment.

Interestingly, replacing those Thread()s with multiprocessing.Process() does not trigger the error, but I highly doubt that's behaving properly.

[v0lat1le]

Yeah, env2 seems to resolve the issue, thank you! Hoping it'll get merged in soon :-)

As a side note I have a plausible use case that will not work as expected

import rasterio
from concurrent.futures import ThreadPoolExecutor
from collections import namedtuple

Info = namedtuple('Info', ['uri', 'username', 'password'])

def get_data(uri):
    with rasterio.Env(GDAL_HTTP_USERPWD="%s:%s"%(uri.username, uri.password)):
        with rasterio.open(uri.uri) as src:
            return src.read()

uris = [Info('http://host1/file.tif', 'user1', 'pass1'), 
        Info('http://host2/file.tif', 'user2', 'pass2'), ...]

pool = ThreadPoolExecutor(8)
data = list(pool.map(get_data, uris))

That use case is covered by CPLSetThreadLocalConfigOption in GDAL. Incidentally it appears that fiona is using that method over CPLSetConfigOption in its variant of GDALEnv

I think thread local solution provides the most intuitive and coherent behaviour as long as it's clearly documented that each thread starts with a 'clean slate'. Definitely clearer then Thread 2 inheriting Thread 1's environment (or the other way round depending on the scheduler's mood today ;-P)

[geowurster]

@v0lat1le That's a great test case. I'll see if I can't get env2 in a reviewable state this weekend and then we can sort this out post-merge.

[sgillies]

@geowurster @v0lat1le I'm game to try making the global _env a threading.local and use CPL's thread local functions to sync it with GDAL. A clean slate is useful if your threads have different duties, which is a pretty standard use case, yes?

[geowurster]

@sgillies That seems doable.

[cmdelatorre]

I've run into this same issue. I'm happy to know you are working on it.

[sgillies]

Closed via #997.

[cmdelatorre]

Do you have an ETA for the next release which could include this improvement?

[geowurster]

@cmdelatorre Already underway in https://github.com/mapbox/rasterio/issues/1004.

[sgillies]

Yes, I'm warming up the Makefile now, will have wheels on PyPI in a few hours.