You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I recently came across a problem where LG_init_has_been_called (defined as a global variable in src/utility/LAGr_init.c) isn't set to false in between subtests in test_BreadthFirstSearch which triggers a cascade of problems which were somewhat tough to track down.
I can only reproduce this error with a modified version of clang 6.0.0 where LAGraph is built with LAGRAPH_VANILLA=1 and linked to Lucata's implementation of GraphBLAS).
Details
This is somewhat convoluted to describe so please ask questions if the following isn't clear. When LAGr_init() fails, it calls GrB_finalize, but since the error in LAGraph_Init() isn't caught inside setup() (see here), the subtest proceeds. It will then try to call GrB_finalize() again (if it doesn't crash earlier) which leads to freeing already freed memory.
Proposed Solution
Primary Change: add extern bool LG_init_has_been_called; to LAGraph_Finalize.c and set it to false when LAGraph_Finalize() completes successfully
Minor change: Wrap LAGraph_Init and LAGraph_Finalize in checks to make sure the user knows if they fail.
I'm happy to open a PR with these changes, but I wanted to discuss them first. Let me know what you think.
The text was updated successfully, but these errors were encountered:
I talked this over with the LAGraph group. We have a different proposed solution.
First of all, I moved the setting of this flag (now called LG_LAGr_Init_has_been_called) to occur at the very end of LAGr_init. This will help in the rare but possible user situation where LAGr_Init fails.
I also added two functions to set/get this flag (see LAGr_Init.c in the v1.1_branch). In normal usage, the flag should only be set by LAGr_Init. It should not be cleared by LAGraph_Finalize, as you suggest, however.
But what you can do is what we have in test_Xinit. That method adds the prototypes of these two set/get functions, so it can clear and set the flag as needed for its tests. In particular, the test_Xinit_brutal method tests LAGr_Init many times, all with pretend failures of running out of memory, and successfully recovers.
LAGraph_Finalize can safely free any partially allocated blocks of memory that were allocated by LAGr_Init.
So if you'd like control over this flag in your tests, just add the prototypes to your tests:
General
I recently came across a problem where
LG_init_has_been_called
(defined as a global variable insrc/utility/LAGr_init.c
) isn't set to false in between subtests intest_BreadthFirstSearch
which triggers a cascade of problems which were somewhat tough to track down.I can only reproduce this error with a modified version of clang 6.0.0 where LAGraph is built with
LAGRAPH_VANILLA=1
and linked to Lucata's implementation of GraphBLAS).Details
This is somewhat convoluted to describe so please ask questions if the following isn't clear. When
LAGr_init()
fails, it callsGrB_finalize
, but since the error inLAGraph_Init()
isn't caught insidesetup()
(see here), the subtest proceeds. It will then try to callGrB_finalize()
again (if it doesn't crash earlier) which leads to freeing already freed memory.Proposed Solution
extern bool LG_init_has_been_called;
toLAGraph_Finalize.c
and set it to false whenLAGraph_Finalize()
completes successfullyLAGraph_Init
andLAGraph_Finalize
in checks to make sure the user knows if they fail.I'm happy to open a PR with these changes, but I wanted to discuss them first. Let me know what you think.
The text was updated successfully, but these errors were encountered: