You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@guacamole.apache.org by "Stephen L. De Rudder (Jira)" <ji...@apache.org> on 2025/06/13 05:43:00 UTC

[jira] [Updated] (GUACAMOLE-2083) guac_rwlock_acquire_write_lock is called after guac_rwlock_destroy is called on a lock

     [ https://issues.apache.org/jira/browse/GUACAMOLE-2083?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Stephen L. De Rudder updated GUACAMOLE-2083:
--------------------------------------------
    Description: 
guac_display_free does:
    guac_rwlock_destroy(&display->last_frame.lock);
    guac_rwlock_destroy(&display->pending_frame.lock);

then calls:
    guac_display_free_layer(display->pending_frame.layers);
    guac_display_free_layer(display->last_frame.layers);

and in guac_display_free_layer it calls:
    guac_display_remove_layer(display_layer);

that then does
    guac_display* display = display_layer->display;

    /*
     * Remove layer from pending frame
     */

    guac_rwlock_acquire_write_lock(&display->pending_frame.lock);

but display->pending_frame.lock has been destroyed.

Fix is:

{{void guac_display_free(guac_display* display) {}}

{{&nbsp;&nbsp;&nbsp;&nbsp;guac_display_stop(display);}}

{{&nbsp;&nbsp;&nbsp;&nbsp;/* All locks, FIFOs, etc. are now unused and can be safely destroyed */}}
{{&nbsp;&nbsp;&nbsp;&nbsp;guac_flag_destroy(&display->render_state);}}
{{&nbsp;&nbsp;&nbsp;&nbsp;guac_fifo_destroy(&display->ops);}}

{{&nbsp;&nbsp;&nbsp;&nbsp;/* Free all layers within the pending_frame list (NOTE: This will also free}}
{{&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;* those layers from the last_frame list) */}}
{{&nbsp;&nbsp;&nbsp;&nbsp;while (display->pending_frame.layers != NULL)}}
{{&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;guac_display_free_layer(display->pending_frame.layers);}}

{{&nbsp;&nbsp;&nbsp;&nbsp;/* Free any remaining layers that were present only on the last_frame list}}
{{&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;* and not on the pending_frame list */}}
{{&nbsp;&nbsp;&nbsp;&nbsp;while (display->last_frame.layers != NULL)}}
{{&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;guac_display_free_layer(display->last_frame.layers);}}

{{&nbsp;&nbsp;&nbsp;&nbsp;guac_rwlock_destroy(&display->last_frame.lock); // MOVED}}
{{&nbsp;&nbsp;&nbsp;&nbsp;guac_rwlock_destroy(&display->pending_frame.lock); // MOVED}}

{{&nbsp;&nbsp;&nbsp;&nbsp;guac_mem_free(display);}}
{{\}}}


Thanks,
SLDR
(Stephen L. De Rudder)

  was:
guac_display_free does:
    guac_rwlock_destroy(&display->last_frame.lock);
    guac_rwlock_destroy(&display->pending_frame.lock);

then calls:
    guac_display_free_layer(display->pending_frame.layers);
    guac_display_free_layer(display->last_frame.layers);

and in guac_display_free_layer it calls:
    guac_display_remove_layer(display_layer);

that then does
    guac_display* display = display_layer->display;

    /*
     * Remove layer from pending frame
     */

    guac_rwlock_acquire_write_lock(&display->pending_frame.lock);

but display->pending_frame.lock has been destroyed.

Fix is:

void guac_display_free(guac_display* display) {

    guac_display_stop(display);

    /* All locks, FIFOs, etc. are now unused and can be safely destroyed */
    guac_flag_destroy(&display->render_state);
    guac_fifo_destroy(&display->ops);

    /* Free all layers within the pending_frame list (NOTE: This will also free
     * those layers from the last_frame list) */
    while (display->pending_frame.layers != NULL)
        guac_display_free_layer(display->pending_frame.layers);

    /* Free any remaining layers that were present only on the last_frame list
     * and not on the pending_frame list */
    while (display->last_frame.layers != NULL)
        guac_display_free_layer(display->last_frame.layers);

    guac_rwlock_destroy(&display->last_frame.lock);
    guac_rwlock_destroy(&display->pending_frame.lock);

    guac_mem_free(display);

}

 

Thanks,
SLDR
(Stephen L. De Rudder)


> guac_rwlock_acquire_write_lock is called after guac_rwlock_destroy is called on a lock
> --------------------------------------------------------------------------------------
>
>                 Key: GUACAMOLE-2083
>                 URL: https://issues.apache.org/jira/browse/GUACAMOLE-2083
>             Project: Guacamole
>          Issue Type: Bug
>          Components: guacd
>    Affects Versions: 1.6.0
>            Reporter: Stephen L. De Rudder
>            Priority: Minor
>
> guac_display_free does:
>     guac_rwlock_destroy(&display->last_frame.lock);
>     guac_rwlock_destroy(&display->pending_frame.lock);
> then calls:
>     guac_display_free_layer(display->pending_frame.layers);
>     guac_display_free_layer(display->last_frame.layers);
> and in guac_display_free_layer it calls:
>     guac_display_remove_layer(display_layer);
> that then does
>     guac_display* display = display_layer->display;
>     /*
>      * Remove layer from pending frame
>      */
>     guac_rwlock_acquire_write_lock(&display->pending_frame.lock);
> but display->pending_frame.lock has been destroyed.
> Fix is:
> {{void guac_display_free(guac_display* display) {}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;guac_display_stop(display);}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;/* All locks, FIFOs, etc. are now unused and can be safely destroyed */}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;guac_flag_destroy(&display->render_state);}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;guac_fifo_destroy(&display->ops);}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;/* Free all layers within the pending_frame list (NOTE: This will also free}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;* those layers from the last_frame list) */}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;while (display->pending_frame.layers != NULL)}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;guac_display_free_layer(display->pending_frame.layers);}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;/* Free any remaining layers that were present only on the last_frame list}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;* and not on the pending_frame list */}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;while (display->last_frame.layers != NULL)}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;guac_display_free_layer(display->last_frame.layers);}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;guac_rwlock_destroy(&display->last_frame.lock); // MOVED}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;guac_rwlock_destroy(&display->pending_frame.lock); // MOVED}}
> {{&nbsp;&nbsp;&nbsp;&nbsp;guac_mem_free(display);}}
> {{\}}}
> Thanks,
> SLDR
> (Stephen L. De Rudder)



--
This message was sent by Atlassian Jira
(v8.20.10#820010)