Skip to content

Commit

Permalink
[GL] Fix update issue in OrderedCommand
Browse files Browse the repository at this point in the history
This commit addresses two issues with OrderedCommand:

 - Removing a command did not remove the OrderedCommand
   from its outputs. This could lead to the command being
   updated after being removed and freed.

 - In some concurrent scenarios a removed command may be
   marked dirty and updated even if the OrderedCommand is
   removed from its outputs.
  • Loading branch information
hyazinthh committed Jun 22, 2023
1 parent 8037659 commit 6a5c280
Showing 1 changed file with 31 additions and 24 deletions.
55 changes: 31 additions & 24 deletions src/Aardvark.Rendering.GL/Management/PreparedRenderObject.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1546,11 +1546,15 @@ module rec Command =
let reader = o.GetReader()

let mutable state : IndexList<ProgramFragment * Command> = IndexList.empty
let mutable dirty = System.Collections.Generic.HashSet<Command>()

override x.InputChangedObject(t : obj, i : IAdaptiveObject) =
match i with
| :? Command as c -> lock dirty (fun () -> dirty.Add c |> ignore)
let dirty = LockedSet<Command>()

let isValid (cmd : Command) =
state |> IndexList.exists (fun _ -> snd >> (=) cmd)

override x.InputChangedObject(_ : obj, input : IAdaptiveObject) =
match input with
| :? Command as cmd -> dirty.Add cmd |> ignore
| _ -> ()

override x.PerformUpdate(token : AdaptiveToken, program : FragmentProgram, info : CompilerInfo) =
Expand All @@ -1562,56 +1566,60 @@ module rec Command =
| Set cmd ->
cmd.Update(token, info)

let fragment =
let fragment =
match s with
| Some (fragment, old) ->
old.Free info
lock dirty (fun () -> dirty.Remove old |> ignore)
old.Outputs.Remove x |> ignore
dirty.Remove old |> ignore

fragment.Mutate(fun s p ->
s.Call(cmd.Program.Value, p)
)
fragment

| None ->
let fragment =
let fragment =
program.NewFragment (fun s p ->
s.Call(cmd.Program.Value, p)
)

match r with
| Some (_, (r, _)) -> fragment.Next <- Some r
| None -> fragment.Next <- None

match l with
| Some (_, (lf,_)) -> lf.Next <- Some fragment
| None -> program.First <- Some fragment




fragment

state <- IndexList.set i (fragment, cmd) state

| Remove ->
match s with
| Some (oldFragment, oldCmd) ->
oldCmd.Free info
oldCmd.Outputs.Remove x |> ignore
dirty.Remove oldCmd |> ignore
oldFragment.Dispose()
lock dirty (fun () -> dirty.Remove oldCmd |> ignore)

| None ->
()

state <- IndexList.remove i state


let mine =
lock dirty (fun () ->
let d = Seq.toArray dirty
dirty.Clear()
d
)

for m in mine do
m.Update(token, info)
// Update all dirty commands
// Note: In concurrent situations, the dirty set may contain already removed commands.
// We have to check if the dirty command is actually still valid before updating it.
//
// Example with threads A and B:
// A: Commits transaction, invokes InputChangedObject(_, cmd)
// B: Removes and frees cmd
// A: Adds cmd to dirty set
// B: Retrieves cmd from dirty set and updates it -> fail
for cmd in dirty.GetAndClear() do
if isValid cmd then cmd.Update(token, info)

override x.Free(info : CompilerInfo) =
for (f, cmd) in state do
Expand All @@ -1620,7 +1628,6 @@ module rec Command =

state <- IndexList.empty
dirty.Clear()
()

type ClearCommand(signature : IFramebufferSignature, values : aval<ClearValues>) =
inherit Command()
Expand Down

0 comments on commit 6a5c280

Please sign in to comment.