Skip to content

Commit

Permalink
Dart: allow conditionally disabling 'Finalizable' marker usage (#1646)
Browse files Browse the repository at this point in the history
---------- Motivation ----------
It turned out that in certain rare cases
Dart compiler experiences internal compiler
error related to scope building when 'Finalizable'
is used.

The fix for the problems is available in Dart SDK 3.6.0+.
There can be problems if the user cannot immediately
upgrade to the new Dart SDK and wants to use other
new features/bug-fixes.

---------- Implemented change ----------
In order to ease the transition phase for users the
new CLI flag called 'dartdisablefinalizablemarker' was
introduced. It allows users to conditionally disable
the usage of 'Finalizable' marker interface.

Moreover, new CMake tests have been introduced to verify
that 'GLUECODIUM_DART_DISABLE_FINALIZABLE_MARKER'
flag works as expected. They bring also a new utility for CMake
tests, which allow users to verify if the generated file contains a
given string.

---------- IMPORTANT ----------
It is discouraged to use this flag, because when 'Finalizable'
marker interface is not used a rare race conditions between
garbage collection and native finalizers may occur.

The flag is designed ONLY to help during the transition phase to
Dart SDK 3.6.0+.

Signed-off-by: Patryk Wrobel <183546751+pwrobeldev@users.noreply.github.com>
  • Loading branch information
pwrobeldev authored Jan 15, 2025
1 parent f4393fb commit 6519c60
Show file tree
Hide file tree
Showing 16 changed files with 233 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased
### Bug fixes:
* Dart: introduced `GLUECODIUM_DART_DISABLE_FINALIZABLE_MARKER` variable as well as `-dartdisablefinalizablemarker` CLI flag, which allows conditionally disabling the usage of `Finalizable` interface from Dart:FFI. It was needed, because the fix introduced in Gluecodium's release `13.10.1` in some rare cases could cause internal compiler issue in Dart SDK (solved in Dart SDK 3.6.0+). This flag is intended to ease the transition to Dart 3.6.0+.
* Dart: fixed a bug related to compilation error caused by usage of `@PositionalDefaults` and default value for a field that uses type, which does not provide const constructor.
* Dart: disabled generation of `invalid_use_of_visible_for_testing_member` warning from the generated code for a hierarchy of classes.
* Dart: removed generation of redundant import for constants declared with external types. The redundant import caused linter warnings.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,14 @@ _gluecodium_define_target_property(
"This property is initialized by the value of the GLUECODIUM_DART_FUNCTION_LOOKUP_ERROR_MESSAGE_DEFAULT variable if it is set when the function gluecodium_add_generate_command is called."
)

_gluecodium_define_target_property(
GLUECODIUM_DART_DISABLE_FINALIZABLE_MARKER
BRIEF_DOCS "Disables usage of Dart:FFI 'Finalizable' marker by the generated types"
FULL_DOCS
"Disables usage of Dart:FFI 'Finalizable' marker by the generated types"
"This property is initialized by the value of the GLUECODIUM_DART_DISABLE_FINALIZABLE_MARKER_DEFAULT variable if it is set when the function gluecodium_add_generate_command is called."
)

_gluecodium_define_target_property(
GLUECODIUM_DART_NAMERULES
BRIEF_DOCS "The path to a file with name rules for Dart"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ function(_prepare_gluecodium_config_file file_path)
_append_boolean_value(validate "${GLUECODIUM_VALIDATE_ONLY}")
_append_boolean_value(swiftexpose "${GLUECODIUM_SWIFT_EXPOSE_INTERNALS}")
_append_boolean_value(strict "${GLUECODIUM_STRICT_VALIDATION}")
_append_boolean_value(dartdisablefinalizablemarker "${GLUECODIUM_DART_DISABLE_FINALIZABLE_MARKER}")

_append_list_option(generators GLUECODIUM_GENERATORS)
_append_list_option(werror GLUECODIUM_WERROR)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Copyright (C) 2025 HERE Europe B.V.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
# SPDX-License-Identifier: Apache-2.0
# License-Filename: LICENSE

cmake_minimum_required(VERSION 3.12)

project(gluecodium.test)
set(CMAKE_CXX_STANDARD 17)

list(APPEND CMAKE_MODULE_PATH "${GLUECODIUM_CMAKE_DIR}/modules")
include(gluecodium/Gluecodium)

include(${GLUECODIUM_CMAKE_TESTS_DIR}/utils/check_file_does_not_contain_string_after_build.cmake)

add_library(dummySharedLibrary SHARED "${CMAKE_CURRENT_LIST_DIR}/cpp/FooImpl.cpp")

gluecodium_generate(dummySharedLibrary GENERATORS cpp dart)
gluecodium_target_lime_sources(dummySharedLibrary
PRIVATE "${CMAKE_CURRENT_LIST_DIR}/lime/foo.lime")

set_target_properties(dummySharedLibrary PROPERTIES GLUECODIUM_DART_DISABLE_FINALIZABLE_MARKER "ON")

set(GENERATED_MAIN_DIR_PATH "${CMAKE_CURRENT_BINARY_DIR}/gluecodium/dummySharedLibrary/cpp-dart/main")
set(GENERATED_MAIN_DART_FILES_PATH "${GENERATED_MAIN_DIR_PATH}/dart/lib/src/unit/test")

check_file_does_not_contain_string_after_build(
dummySharedLibrary "${GENERATED_MAIN_DART_FILES_PATH}/foo.dart" "Finalizable")

check_file_does_not_contain_string_after_build(
dummySharedLibrary "${GENERATED_MAIN_DART_FILES_PATH}/bar.dart" "Finalizable")

check_file_does_not_contain_string_after_build(
dummySharedLibrary "${GENERATED_MAIN_DART_FILES_PATH}/baz.dart" "Finalizable")
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// -------------------------------------------------------------------------------------------------
// Copyright (C) 2016-2025 HERE Europe B.V.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// SPDX-License-Identifier: Apache-2.0
// License-Filename: LICENSE
//
// -------------------------------------------------------------------------------------------------
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Copyright (C) 2016-2025 HERE Europe B.V.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
# SPDX-License-Identifier: Apache-2.0
# License-Filename: LICENSE

package unit.test

class Foo {

}

interface Bar {
fun bar()
}

lambda Baz = (Int) -> Void
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Copyright (C) 2016-2025 HERE Europe B.V.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
# SPDX-License-Identifier: Apache-2.0
# License-Filename: LICENSE

#[=======================================================================[.rst:
.. command:check_file_does_not_contain_string_after_build
Checks that provided file does not contain a given string after given target is built.
Adds post build command which tries to grep for a given string.
check_file_does_not_contain_string_after_build(
<target> # Target to add post build command
<file_path> # File or directory path to check
<needle> # String to look for in the file.
)
#]=======================================================================]

function(check_file_does_not_contain_string_after_build _target file_path needle)
add_custom_command(
TARGET ${_target}
POST_BUILD
COMMAND ${CMAKE_COMMAND} -E echo
"Loads a file '${file_path}' and looks for '${needle}'. This file contains it if command fails."
COMMAND ${CMAKE_COMMAND} -DCHECK_FILE_DOES_NOT_CONTAIN_FILE_PATH="${file_path}"
-DCHECK_FILE_DOES_NOT_CONTAIN_NEEDLE="${needle}"
-P "${GLUECODIUM_CMAKE_TESTS_DIR}/utils/run_check_file_does_not_contain_string.cmake")
endfunction()
38 changes: 38 additions & 0 deletions cmake/tests/utils/run_check_file_does_not_contain_string.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Copyright (C) 2016-2025 HERE Europe B.V.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
# SPDX-License-Identifier: Apache-2.0
# License-Filename: LICENSE

cmake_minimum_required(VERSION 3.5 FATAL_ERROR)

include("${CMAKE_CURRENT_LIST_DIR}/message_colored.cmake")

set(_required_vars CHECK_FILE_DOES_NOT_CONTAIN_FILE_PATH CHECK_FILE_DOES_NOT_CONTAIN_NEEDLE)

foreach(_required_variable ${_required_vars})
if(NOT DEFINED ${_required_variable})
message_colored(RED FATAL_ERROR "${_required_variable} must be specified")
endif()
endforeach()

file(STRINGS ${CHECK_FILE_DOES_NOT_CONTAIN_FILE_PATH} _lines)

foreach(_line ${_lines})
if("${_line}" MATCHES "${CHECK_FILE_DOES_NOT_CONTAIN_NEEDLE}")
message_colored(
RED FATAL_ERROR
"The file ${CHECK_FILE_DOES_NOT_CONTAIN_FILE_PATH} contains: ${CHECK_FILE_DOES_NOT_CONTAIN_NEEDLE}")
endif()
endforeach()
10 changes: 10 additions & 0 deletions gluecodium/src/main/java/com/here/gluecodium/cli/OptionReader.kt
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,15 @@ object OptionReader {
addOption("internalprefix", true, "Name prefix for internal conversion functions in Swift.")
addOption("libraryname", true, "Name of the generated library for some generators (e.g. Dart).")
addOption("dartlookuperrormessage", true, "Custom error message for when Dart FFI function lookup fails.")
addOption(
"dartdisablefinalizablemarker",
false,
"Disables usage of Dart:FFI 'Finalizable' marker by the generated types. " +
"Warning: this is discouraged as it may lead to rare race conditions between native finalizers and " +
"garbage collection. It was designed to be used only during transition to newer Dart SDK (versions " +
" below 3.6.0. may experience internal compiler error when compiling the generated code, which utilizes " +
"'Finalizable' marker)",
)
addOption(
"werror",
"warning-as-error",
Expand Down Expand Up @@ -203,6 +212,7 @@ object OptionReader {
getStringValue("dartlookuperrormessage")?.let { generatorOptions.dartLookupErrorMessage = it }
getStringListValue("werror")?.let { generatorOptions.werror = it.toSet() }

generatorOptions.dartDisableFinalizableMarker = getFlagValue("dartdisablefinalizablemarker")
generatorOptions.swiftExposeInternals = getFlagValue("swiftexpose")

generatorOptions.cppNameRules = readConfigFile(getStringValue("cppnamerules"), generatorOptions.cppNameRules)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ data class GeneratorOptions(
var cppExportCommon: String? = null,
var internalPrefix: String? = null,
var libraryName: String = "library",
var dartDisableFinalizableMarker: Boolean = false,
var dartLookupErrorMessage: String =
"Failed to resolve an FFI function. Perhaps `LibraryContext.init()` was not called.",
var swiftExposeInternals: Boolean = false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ internal class DartGenerator : Generator {
private lateinit var commentsProcessor: CommentsProcessor
private lateinit var activeTags: Set<String>
private var overloadsWerror: Boolean = false
private var disableFinalizableMarker: Boolean = false

override val shortName = "dart"

Expand All @@ -101,6 +102,7 @@ internal class DartGenerator : Generator {
internalPrefix = options.internalPrefix ?: ""
commentsProcessor = DartCommentsProcessor(options.werror.contains(GeneratorOptions.WARNING_DOC_LINKS))
overloadsWerror = options.werror.contains(GeneratorOptions.WARNING_DART_OVERLOADS)
disableFinalizableMarker = options.dartDisableFinalizableMarker
activeTags = options.tags
}

Expand Down Expand Up @@ -267,6 +269,7 @@ internal class DartGenerator : Generator {
"optimizedLists" to optimizedLists,
"descendantInterfaces" to descendantInterfaces,
"asyncHelpers" to asyncHelpers,
"disableFinalizableMarker" to disableFinalizableMarker,
),
nameResolvers,
predicates,
Expand Down Expand Up @@ -401,6 +404,7 @@ internal class DartGenerator : Generator {
"lookupErrorMessage" to lookupErrorMessage,
"builtInTypes" to TypeId.values().subtract(customNullableTypes),
"typeRepositories" to typeRepositories.sortedBy { it.fullName },
"disableFinalizableMarker" to disableFinalizableMarker,
"imports" to
typeRepositories.flatMap { importResolver.resolveElementImports(it) }.distinct().sorted(),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@
!}}
{{>dart/DartDocumentation}}{{>dart/DartAttributes}}
abstract class {{resolveName}}{{!!
}} implements {{#if this.parents}}{{#this.parents}}{{resolveName}}, {{/this.parents}}{{/if}}Finalizable {
}}{{#unless disableFinalizableMarker}}{{!!
}} implements {{#if this.parents}}{{#this.parents}}{{resolveName}}, {{/this.parents}}{{/if}}Finalizable{{!!
}}{{/unless}}{{!!
}}{{#if disableFinalizableMarker}}{{!!
}}{{#if this.parents}} implements {{#this.parents}}{{resolveName}}{{#if iter.hasNext}}, {{/if}}{{/this.parents}}{{/if}}{{!!
}}{{/if}} {
{{#set parent=this container=this}}{{#constructors}}
{{prefixPartial "dart/DartFunctionDocs" " "}}
factory {{resolveName parent}}{{>dart/DartConstructorName}}({{!!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@
!}}
{{>dart/DartDocumentation}}{{>dart/DartAttributes}}
abstract class {{resolveName}}{{!!
}} implements {{#if this.parents}}{{#this.parents}}{{resolveName}}, {{/this.parents}}{{/if}}Finalizable {
}}{{#unless disableFinalizableMarker}}{{!!
}} implements {{#if this.parents}}{{#this.parents}}{{resolveName}}, {{/this.parents}}{{/if}}Finalizable{{!!
}}{{/unless}}{{!!
}}{{#if disableFinalizableMarker}}{{!!
}}{{#if this.parents}} implements {{#this.parents}}{{resolveName}}{{#if iter.hasNext}}, {{/if}}{{/this.parents}}{{/if}}{{!!
}}{{/if}} {
{{#if inheritedFunctions functions inheritedProperties properties logic="or"}}
{{prefixPartial "dart/DartDocumentation" " "}}
factory {{resolveName}}(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,12 @@ final _{{resolveName "Ffi"}}CreateProxy = __lib.catchArgumentError(() => __lib.n
Pointer<Void> Function(int, int, Object, Pointer)
>('{{libraryName}}_{{resolveName "FfiSnakeCase"}}_create_proxy'));

class {{resolveName}}$Impl implements Finalizable {
{{#unless disableFinalizableMarker}}{{!!
}}class {{resolveName}}$Impl implements Finalizable {
{{/unless}}{{!!
}}{{#if disableFinalizableMarker}}{{!!
}}class {{resolveName}}$Impl {
{{/if}}
final Pointer<Void> handle;
{{resolveName}}$Impl(this.handle);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,12 @@ class _LazyIterator<E> implements Iterator<E> {
}

/// @nodoc
class LazyList<E> extends Iterable<E> implements List<E>, Finalizable {
{{#unless disableFinalizableMarker}}{{!!
}}class LazyList<E> extends Iterable<E> implements List<E>, Finalizable {
{{/unless}}{{!!
}}{{#if disableFinalizableMarker}}{{!!
}}class LazyList<E> extends Iterable<E> implements List<E> {
{{/if}}
static final _cannotModify = "Cannot modify an unmodifiable list";

final Pointer<Void> handle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,18 @@ class OptionReaderTest {
assertEquals("", options!!.second.copyrightHeaderContents)
}

@Test
fun dartdisablefinalizablemarkerFlagIsProperlyRead() {
// Arrange
val commandLineParameters = arrayOf("-dartdisablefinalizablemarker")

// Act
val options = OptionReader.read(commandLineParameters)

// Assert
assertTrue(options!!.second.dartDisableFinalizableMarker)
}

private fun prepareToRead(
optionName: String,
optionValue: String,
Expand Down

0 comments on commit 6519c60

Please sign in to comment.