Binding code releases memory too early

[stevehartdata]

I'm fairly new to Haskell, so please correct me if any of this is incorrect. I hope to submit a pull request once I've produced a clean fix. In the meantime, this is just so that others don't have to spend the time re-diagnosing the problem, and so that I have some notes to refresh my memory in case I can't fix the problem right away.

Summary

Running commands and queries with bound data has inconsistent results. Sometimes the statements succeed, while at other times they result in one of the following errors:

• ORA-01461: can bind a LONG value only for insert into a LONG column
  • This occurs even though none of the bound values are of type LONG and none of the strings come close to exceeding Oracle's 4000 character limit.
• NULL constraint violations from bound data that is not actually NULL.
• Memory access violations resulting in program crashes.

Cause

The inconsistent results occur because the memory used to hold binding-related data can be freed before a statement is executed.

Data is basically bound by calls to bindP. The implementation for most data types looks like this:

takusen-oracle/Database/Oracle/Enumerator.hs

Line 585 in 221302e

bindP = makeBindAction

makeBindAction produces a BindA that contains a function Session -> PreparedStmtObj -> Int -> IO (). The Int is the position of the bind variable in the statement. Here's the relevant code:

takusen-oracle/Database/Oracle/Enumerator.hs

Lines 650 to 656 in 221302e

	makeBindAction x = BindA (\ses st -> bindMaybe ses st x)
	bindMaybe :: (OracleBind a)
	=> Session -> PreparedStmtObj -> Maybe a -> Int -> IO ()
	bindMaybe sess stmt v pos =
	bindWithValue v $ \ptrv -> do
	bindByPos sess stmt pos (bindNullInd v) (castPtr ptrv) (bindDataSize v) (bindType v)

makeBindAction calls bindMaybe, which calls bindByPos through a wrapper. bindByPos calls mallocForeignPtr and newForeignPtr_ to allocate memory to hold data that needs to be passed to the OCI function.

takusen-oracle/Database/Oracle/OCIFunctions.hs

Lines 555 to 565 in 221302e

	bindByPos err stmt pos nullInd bufptr sze sqltype = do
	indFPtr <- mallocForeignPtr
	sizeFPtr <- mallocForeignPtr
	withForeignPtr indFPtr $ \p -> poke p nullInd
	-- You can't put any old junk in the return-size field,
	-- even if the parameter is IN-only.
	-- So tell it how big the input buffer is.
	withForeignPtr sizeFPtr $ \p -> poke p (fromIntegral sze)
	bufFPtr <- newForeignPtr_ bufptr
	bindOutputByPos err stmt pos (indFPtr, bufFPtr, sizeFPtr) sze sqltype
	return ()

This is where the problem is.

According to the Oracle OCI documentation, "Program input data does not need to be in the program variable when it is bound to the placeholder, but the data must be there when the statement is executed."

mallocForeignPtr returns a ForeignPtr to some memory that "will be released automatically when the ForeignPtr is discarded" (see the documentation).

These ForeignPtrs are not returned by bindByPos, so as far as I can tell, they will be discarded as soon as the IO action to bind the variables is executed (line 575 below). After that, the memory can be released at any time. Note that this is before the statement is executed, so the bind data may not be available at the required time (line 579 below).

takusen-oracle/Database/Oracle/Enumerator.hs

Lines 573 to 581 in 221302e

	instance IPrepared PreparedStmtObj Session BoundStmt BindObj where
	bindRun sess stmt bas action = do
	sequence_ (zipWith (\i (BindA ba) -> ba sess stmt i) [1..] bas)
	let iteration = case (stmtType stmt) of
	SelectType -> 0
	CommandType -> 1
	execute sess (stmtHandle stmt) iteration
	writeIORef (stmtCursors stmt) []
	action (BoundStmt stmt)

I think bindByPos also may be leaking memory, since I don't see the memory allocated by newForeignPtr_ released anywhere. (newForeignPtr_ does not associate any finalizers with the ForeignPtr.) I'll open a separate issue for that later if it turns out to be the case.

Next Steps

I found these issues when implementing array binding, and I worked around them by allocating memory in bindPyPos and returning a IO [IO ()] with the IO actions needed to free the memory that was allocated. I threaded these "finalizers" up through bindRun so that they could be called after the statement is executed. It looks kind of like this in my quick fix:

  bindRun sess stmt bas action = do
    finalizers <- (sequence (zipWith (\i (BindA ba) -> ba sess stmt i) [1..] bas) :: IO [[IO ()]])
    let iteration = case (stmtType stmt) of
          SelectType -> 0
          (CommandType itCount) -> itCount
    execute sess (stmtHandle stmt) iteration
    writeIORef (stmtCursors stmt) []
    r <- action (BoundStmt stmt)
    sequence_ . concat $ finalizers
    return r

I think there is likely a cleaner solution, but I haven't yet figured out what it is.

[paulrzcz]

Thanks a lot for the detailed bug report! And sorry for the late response.

It's cool that you found the quick fix. That means that I've got time for fixing the library. I'll try to investigate the issue and come back with a solution.

[stevehartdata]

Just FYI, I've been working on this when I've had some time. I've been doing some cleanup work to get the tests to run so that I'll be able to verify that I haven't broken things with the changes to address this issue. You can see the progress at stevehartdata/takusen-oracle@f756de4, in case it's helpful to you. I plan to package it up in a pull request once I've fixed this issue.

I've found that this issue is likely to arise when binding long strings. If I understand correctly, the block containing the pinned memory allocated by mallocForeignPtr won't be collected until it is filled up, so you can increase the chance that it will be collected before the statement is executed by binding data that exceeds the block size. Of course there are other ways to reproduce the issue, but I think this is the easiest.