Fix eterm tab closing and macOS menu integration

* Serialize tracked realloc and free bookkeeping to prevent cross-thread
    address reuse from corrupting the memory tracker during tab teardown.
  * Add concurrent allocation and terminal close/resize regression coverage.
  * Install a macOS-only File, Edit, and Window menu that mirrors eterm
    keybindings without binding Cmd+W.
  * Keep the native menu outside the splitter layout and document the canonical
    macOS build workflow.
This commit is contained in:
Martín Lucas Golini
2026-09-21 21:39:39 -03:00
parent e2174aa044
commit f200850941
9 changed files with 351 additions and 27 deletions

View File

@@ -4,6 +4,29 @@ The build configurations in `.ecode/project_build.json` are the source of truth
developer's local build workflows. Check that file before selecting a generator, backend, or
build flags. In particular, do not use an AddressSanitizer build to evaluate runtime performance.
## macOS Builds (Required Workflow)
On a macOS host, the scripts in `projects/macos` override the generic Premake and Make instructions
below. They regenerate and build the project with the same options used by the developer and must be
used for debug, release, unit-test, targeted, and clean builds. Do not invoke `premake4`, `premake5`,
or `make -C make/macosx` directly on macOS, because doing so can regenerate the shared build tree
with incompatible options and cause subsequent local builds to fail.
Before building, read `.ecode/project_build.json` and use the configuration and target selected
there. From the repository root, run `projects/macos/make_no_fw.sh` first, forwarding the exact
`config=<build_type>`, target, and action arguments required by the task. Examples:
* Debug: `projects/macos/make_no_fw.sh config=debug`
* Release: `projects/macos/make_no_fw.sh config=release`
* Debug unit tests: `projects/macos/make_no_fw.sh config=debug eepp-unit_tests`
* Release unit tests: `projects/macos/make_no_fw.sh config=release eepp-unit_tests`
* Clean: `projects/macos/make_no_fw.sh config=<build_type> clean`
If `make_no_fw.sh` fails, retry once with `projects/macos/make.sh`, preserving the exact same
arguments. Do not add sanitizer, linker, framework, generator, architecture, or parallelism flags
outside the scripts unless `.ecode/project_build.json` itself requires them. Do not fall back to a
hand-written Premake or direct Make command.
## Release Performance Builds (Linux)
For performance investigations, use the `eepp-linux-ninja` configuration from
@@ -27,12 +50,13 @@ release Ninja build required for performance work.
## Debug and Unit-Test Builds
All build commands must be executed from the **root project directory**. Follow these steps to build the project:
All build commands must be executed from the **root project directory**. On macOS, follow the
required script workflow above. The generic steps below apply to other hosts.
## Step 1: Regenerate Project Files
Always regenerate the project files before compiling or running tests after making changes. Do this even for edits to existing files, because the checked-in makefiles can be stale and may reference removed files or miss recently added targets.
* **Tool:** Use `premake4` if installed; otherwise, fallback to `premake5` (the parameters are identical).
* **Tool:** On non-macOS hosts, use `premake4` if installed; otherwise, fallback to `premake5` (the parameters are identical).
* **Linker Flag (`--with-mold-linker`):** This flag is conditional. If the `mold` linker is installed on the system, you **must** include it to speed up linking. If `mold` is not installed, omit the flag.
Choose the generator command from the configuration that will be built. Debug builds use AddressSanitizer; release builds do not.
@@ -74,11 +98,8 @@ Always use all processors reported by the platform when selecting the parallel j
and other systems with `nproc`, use `-j$(nproc)` exactly; do not substitute an arbitrary fixed value
such as `-j4`. Use the platform-equivalent processor-count command where `nproc` is unavailable.
On macOS in a managed or sandboxed environment, request elevated permission before invoking
`sysctl` or running any command that needs access to host display services. Read and validate
`sysctl -n hw.ncpu` separately before starting the build, then pass the verified positive value as a
literal `-jN` argument. If the query fails, returns zero, or returns an empty value, stop and request
permission; never allow an empty command substitution to turn `-j` into unbounded parallelism.
On macOS, do not query `sysctl` or select a job count separately; the required scripts above own
parallelism and invoke the platform query themselves.
The valid OS directory names are: `windows`, `macosx`, `linux`, `bsd`, `haiku`.
@@ -87,8 +108,7 @@ Run the following command, replacing `<os_name>` with the correct environment:
**Examples:**
* Linux: `make -C make/linux -j$(nproc)`
* macOS: after an approved `sysctl -n hw.ncpu` returns a value such as `10`, run
`make -C make/macosx -j10` using that exact verified value.
* macOS: use `projects/macos/make_no_fw.sh config=<build_type>` as documented above.
* Windows: `make -C make/windows -j%NUMBER_OF_PROCESSORS%`
## Running GUI Examples Under Xvfb

View File

@@ -658,6 +658,12 @@
"command": "${project_root}/bin/eepp-ui-markdownview-debug",
"name": "eepp-ui-markdownview-debug",
"working_dir": "${project_root}/bin/"
},
{
"args": "",
"command": "${project_root}/bin/eterm-debug",
"name": "eterm-debug",
"working_dir": "${project_root}/bin/"
}
],
"var": {

View File

@@ -38,10 +38,16 @@ class EE_API MemoryManager {
static void* reallocPointer( void* data, const AllocatedPointer& aAllocatedPointer );
/** Reallocates a tracked pointer while keeping allocator and bookkeeping state atomic. */
static void* reallocateTracked( void* data, size_t size, const char* file, size_t line );
static void* addPointerInPlace( void* place, const AllocatedPointer& aAllocatedPointer );
static bool removePointer( void* data, const char* file, const size_t& line );
/** Removes a tracked allocation before returning its address to the C allocator. */
static bool freeTracked( void* data, const char* file, size_t line );
/** Removes a pointer when it is tracked, without diagnosing foreign allocator bookkeeping. */
static bool removePointerIfTracked( void* data );
@@ -121,14 +127,8 @@ class EE_API MemoryManager {
EE::MemoryManager::addPointer( EE::AllocatedPointer( EE::MemoryManager::allocate( amount ), \
__FILE__, __LINE__, amount ) )
#if defined( __GNUC__ ) && __GNUC__ >= 12
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wuse-after-free"
#endif
#define eeRealloc( ptr, amount ) \
EE::MemoryManager::reallocPointer( \
ptr, EE::AllocatedPointer( EE::MemoryManager::reallocate( ptr, amount ), __FILE__, \
__LINE__, amount ) )
#define eeRealloc( ptr, amount ) \
EE::MemoryManager::reallocateTracked( ptr, amount, __FILE__, __LINE__ )
#if defined( __GNUC__ ) && __GNUC__ >= 12
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wuse-after-free"
@@ -145,18 +145,12 @@ class EE_API MemoryManager {
#if defined( __GNUC__ ) && __GNUC__ >= 12
#pragma GCC diagnostic pop
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wuse-after-free"
#endif
#define eeFree( data ) \
{ \
if ( EE::MemoryManager::removePointer( EE::MemoryManager::free( data ), __FILE__, \
__LINE__ ) == false ) \
printf( "Deleting at '%s' %d\n", __FILE__, __LINE__ ); \
#define eeFree( data ) \
{ \
if ( !EE::MemoryManager::freeTracked( data, __FILE__, __LINE__ ) ) \
printf( "Deleting at '%s' %d\n", __FILE__, __LINE__ ); \
}
#if defined( __GNUC__ ) && __GNUC__ >= 12
#pragma GCC diagnostic pop
#endif
#else

View File

@@ -162,6 +162,53 @@ void* MemoryManager::reallocPointer( void* data, const AllocatedPointer& aAlloca
return aAllocatedPointer.mData;
}
void* MemoryManager::reallocateTracked( void* data, size_t size, const char* file, size_t line ) {
MemoryManagerScope scope;
auto& state = getMemoryManagerState();
Lock lock( state.allocationMutex );
auto it = state.pointers.find( data );
if ( size == 0 ) {
if ( it != state.pointers.end() ) {
state.totalMemoryUsage -= it->second.mMemory;
state.pointers.erase( it );
}
::free( data );
return nullptr;
}
void* reallocated = ::realloc( data, size );
if ( !reallocated )
return nullptr;
bool track = false;
bool globalAllocation = false;
if ( it != state.pointers.end() ) {
track = it->second.mTrack;
globalAllocation = it->second.mGlobalAllocation;
state.totalMemoryUsage -= it->second.mMemory;
state.pointers.erase( it );
}
AllocatedPointer allocation( reallocated, file, static_cast<int>( line ), size, track,
globalAllocation );
auto result =
state.pointers.insert( AllocatedPointerMap::value_type( reallocated, allocation ) );
if ( !result.second ) {
state.totalMemoryUsage -= result.first->second.mMemory;
result.first->second = allocation;
}
state.totalMemoryUsage += size;
if ( state.peakMemoryUsage < state.totalMemoryUsage )
state.peakMemoryUsage = state.totalMemoryUsage;
if ( size > state.biggestAllocation.mMemory )
state.biggestAllocation = allocation;
if ( !globalAllocation && size > state.biggestNonAnonymousAllocation.mMemory )
state.biggestNonAnonymousAllocation = allocation;
return reallocated;
}
bool MemoryManager::removePointer( void* data, const char* file, const size_t& line ) {
MemoryManagerScope scope;
auto& state = getMemoryManagerState();
@@ -185,6 +232,12 @@ bool MemoryManager::removePointer( void* data, const char* file, const size_t& l
return true;
}
bool MemoryManager::freeTracked( void* data, const char* file, size_t line ) {
bool tracked = removePointer( data, file, line );
::free( data );
return tracked;
}
bool MemoryManager::removePointerIfTracked( void* data ) {
MemoryManagerScope scope;
auto& state = getMemoryManagerState();

View File

@@ -233,6 +233,46 @@ UTEST( eterm_session, resize_and_output_are_serialized ) {
EXPECT_EQ( static_cast<size_t>( 40 * 12 ), snapshot->cells.size() );
}
UTEST( eterm_session, closing_a_tab_resize_keeps_surviving_session_alive ) {
auto survivorPty = std::make_unique<MockPty>();
survivorPty->mCols = 151;
survivorPty->mRows = 52;
for ( int line = 0; line < 200; ++line )
survivorPty->mBuffer += "survivor history " + std::to_string( line ) + "\r\n";
auto survivorProcess = std::make_unique<MockProcess>();
auto survivor =
TerminalSession::create( std::move( survivorPty ), std::move( survivorProcess ), 1000 );
ASSERT_TRUE( waitForSnapshot( survivor, []( const TerminalSnapshot& snapshot ) {
return snapshot.columns == 151 && snapshot.rows == 52 &&
snapshot.historyLength >= 100;
} ) != nullptr );
auto temporaryPty = std::make_unique<MockPty>();
temporaryPty->mCols = 151;
temporaryPty->mRows = 52;
auto temporaryProcess = std::make_unique<MockProcess>();
auto temporary =
TerminalSession::create( std::move( temporaryPty ), std::move( temporaryProcess ), 1000 );
survivor->resize( 151, 50 );
temporary->resize( 151, 50 );
ASSERT_TRUE( waitForSnapshot( survivor, []( const TerminalSnapshot& snapshot ) {
return snapshot.columns == 151 && snapshot.rows == 50;
} ) != nullptr );
ASSERT_TRUE( waitForSnapshot( temporary, []( const TerminalSnapshot& snapshot ) {
return snapshot.columns == 151 && snapshot.rows == 50;
} ) != nullptr );
/* Queue the surviving resize before closing the temporary session so both workers can release
* and reallocate equal-sized terminal rows concurrently. */
survivor->resize( 151, 52 );
temporary.reset();
ASSERT_TRUE( waitForSnapshot( survivor, []( const TerminalSnapshot& snapshot ) {
return snapshot.columns == 151 && snapshot.rows == 52 &&
snapshot.historyLength >= 100;
} ) != nullptr );
}
UTEST( eterm_session, scroll_snapshots_acknowledge_the_latest_ordered_command ) {
auto pty = std::make_unique<MockPty>();
for ( int line = 0; line < 80; ++line )
@@ -2524,6 +2564,50 @@ UTEST( eterm, history_corruption_on_resize ) {
}
}
UTEST( eterm, repeated_resize_keeps_screen_row_ownership ) {
auto pty = std::make_unique<MockPty>();
auto process = std::make_unique<MockProcess>();
auto display = std::make_shared<MockDisplay>();
auto term = TerminalEmulator::create( std::move( pty ), std::move( process ), display, 1000 );
term->resize( 151, 50 );
for ( int line = 0; line < 200; ++line ) {
const std::string text = "terminal output " + std::to_string( line ) + "\r\n";
term->write( text.c_str(), text.size() );
term->update();
}
for ( int iteration = 0; iteration < 100; ++iteration ) {
term->resize( 151, 52 );
term->resize( 151, 50 );
}
const Vector2i finalSize = term->getSize();
EXPECT_EQ( 151, finalSize.x );
EXPECT_EQ( 50, finalSize.y );
EXPECT_TRUE( term->scrollSize() > 0 );
}
UTEST( eterm, history_capacity_tracks_narrow_and_wide_resize ) {
auto pty = std::make_unique<MockPty>();
pty->mCols = 151;
pty->mRows = 50;
auto process = std::make_unique<MockProcess>();
auto display = std::make_shared<MockDisplay>();
auto term = TerminalEmulator::create( std::move( pty ), std::move( process ), display, 10 );
for ( int line = 0; line < 100; ++line ) {
const std::string text = "short " + std::to_string( line ) + "\r\n";
term->write( text.c_str(), text.size() );
term->update();
}
EXPECT_TRUE( term->scrollSize() > 0 );
term->resize( 80, 50 );
term->resize( 151, 50 );
const Vector2i finalSize = term->getSize();
EXPECT_EQ( 151, finalSize.x );
EXPECT_EQ( 50, finalSize.y );
}
UTEST( eterm_search, logical_lines_options_and_cell_mapping ) {
TerminalGlyph first[] = { { 'H' }, { 'e' }, { 'l' }, { 'l' }, { 'o', ATTR_WRAP } };
TerminalGlyph second[] = { { 'W' }, { 'o' }, { 'r' }, { 'l' }, { 'd' } };

View File

@@ -1,5 +1,8 @@
#include "utest.h"
#include <atomic>
#include <eepp/core/memorymanager.hpp>
#include <thread>
#include <vector>
using namespace EE;
@@ -23,6 +26,45 @@ UTEST( MemoryManager, tracesPlainNew ) {
EXPECT_GE( allocated, before + sizeof( Uint64 ) );
EXPECT_EQ( after, before );
}
UTEST( MemoryManager, tracked_reallocation_updates_atomically ) {
const size_t before = MemoryManager::getTotalMemoryUsage();
auto* allocation = static_cast<Uint8*>( eeMalloc( 64 ) );
allocation = static_cast<Uint8*>( eeRealloc( allocation, 256 ) );
allocation[0] = 42;
const size_t reallocated = MemoryManager::getTotalMemoryUsage();
eeFree( allocation );
const size_t after = MemoryManager::getTotalMemoryUsage();
EXPECT_GE( reallocated, before + 256 );
EXPECT_EQ( after, before );
}
UTEST( MemoryManager, concurrent_tracked_allocation_lifecycle ) {
constexpr int ThreadCount = 4;
constexpr int Iterations = 5000;
std::atomic<bool> start{ false };
std::vector<std::thread> workers;
workers.reserve( ThreadCount );
for ( int thread = 0; thread < ThreadCount; ++thread ) {
workers.emplace_back( [&start, thread] {
while ( !start.load( std::memory_order_acquire ) )
std::this_thread::yield();
for ( int iteration = 0; iteration < Iterations; ++iteration ) {
size_t initialSize = 64 + static_cast<size_t>( ( iteration + thread ) % 4 ) * 16;
auto* allocation = static_cast<Uint8*>( eeMalloc( initialSize ) );
allocation = static_cast<Uint8*>( eeRealloc( allocation, initialSize + 64 ) );
allocation[0] = static_cast<Uint8>( iteration );
eeFree( allocation );
}
} );
}
start.store( true, std::memory_order_release );
for ( auto& worker : workers )
worker.join();
EXPECT_TRUE( true );
}
#endif
UTEST( MemoryManager, supportsExpressionAndLegacyNewSyntax ) {

View File

@@ -509,6 +509,114 @@ void App::addTabKeyBindings( UITerminal* terminal ) {
applyKeybindings( terminal );
}
#if EE_PLATFORM == EE_PLATFORM_MACOS
void App::executeMenuCommand( const std::string& command ) {
if ( command == "create-new-terminal" ) {
createNewTerminal();
return;
}
if ( command == "open-settings" ) {
settingsActions->showSettings();
return;
}
if ( command == "open-keybindings" ) {
openKeybindings();
return;
}
auto* widget = tabSplitter ? tabSplitter->getCurWidget() : nullptr;
if ( !widget )
return;
if ( widget->isType( UI_TYPE_TERMINAL ) ) {
widget->asType<UITerminal>()->execute( command );
} else if ( widget->isType( UI_TYPE_CODEEDITOR ) ) {
auto* editor = widget->asType<UICodeEditor>();
editor->getDocument().execute( command, editor );
}
}
void App::syncGlobalMenuKeybindings() {
if ( !menuBar )
return;
KeyBindings bindings( appWindow->getInput() );
bindings.addKeybindsStringUnordered( keybindings );
for ( Uint32 menuIndex = 0; menuIndex < menuBar->getButtonsCount(); ++menuIndex ) {
auto* menu = menuBar->getPopUpMenu( menuIndex );
for ( Uint32 itemIndex = 0; menu && itemIndex < menu->getCount(); ++itemIndex ) {
auto* item = menu->getItem( itemIndex );
if ( item->isType( UI_TYPE_MENUITEM ) && !item->getId().empty() ) {
item->asType<UIMenuItem>()->setShortcutText(
bindings.getCommandKeybindString( item->getId() ) );
}
}
}
}
void App::createGlobalMenuBar() {
menuBar = UIMenuBar::New();
/* The native menu is not part of the visible layout. Keeping it outside mainLayout preserves
* UITabWidgetSplitter's invariant that its base layout owns only the splitter tree. */
menuBar->setParent( scene->getRoot() );
menuBar->setVisible( false );
/* Installing an application-owned menu replaces AppKit's default Close Window item. Do not add
* a Cmd+W item here: eterm intentionally leaves it unbound and closes tabs with the configured
* close-tab shortcut (Cmd+Shift+W by default). */
const auto addCommand = []( UIPopUpMenu* menu, const String& text,
const std::string& command ) {
auto* item = menu->add( text );
item->setId( command );
return item;
};
const auto onItemClicked = [this]( const Event* event ) {
if ( event->getNode()->isType( UI_TYPE_MENUITEM ) )
executeMenuCommand( event->getNode()->getId() );
};
auto* fileMenu = UIPopUpMenu::New();
addCommand( fileMenu, i18n( "new_terminal", "New Terminal" ), "create-new-terminal" );
addCommand( fileMenu, i18n( "close_tab", "Close Tab" ), "close-tab" );
fileMenu->addSeparator();
addCommand( fileMenu, i18n( "settings", "Settings..." ), "open-settings" )
->setMenuRole( MenuRole::Preferences );
addCommand( fileMenu, i18n( "key_bindings", "Keybindings..." ), "open-keybindings" );
fileMenu->on( Event::OnItemClicked, onItemClicked );
menuBar->addMenuButton( i18n( "file", "File" ), fileMenu );
auto* editMenu = UIPopUpMenu::New();
addCommand( editMenu, i18n( "copy", "Copy" ), "terminal-copy" );
addCommand( editMenu, i18n( "paste", "Paste" ), "terminal-paste" );
editMenu->addSeparator();
addCommand( editMenu, i18n( "find_ellipsis", "Find..." ), "terminal-find" );
addCommand( editMenu, i18n( "find_next", "Find Next" ), "terminal-find-next" );
addCommand( editMenu, i18n( "find_previous", "Find Previous" ), "terminal-find-previous" );
editMenu->on( Event::OnItemClicked, onItemClicked );
editMenu->on( Event::OnMenuShow, [this, editMenu]( const Event* ) {
const auto* widget = tabSplitter ? tabSplitter->getCurWidget() : nullptr;
const bool enabled = widget && widget->isType( UI_TYPE_TERMINAL );
for ( Uint32 i = 0; i < editMenu->getCount(); ++i )
editMenu->getItem( i )->setEnabled( enabled );
} );
menuBar->addMenuButton( i18n( "edit", "Edit" ), editMenu );
auto* windowMenu = UIPopUpMenu::New();
windowMenu->setMenuBarRole( MenuBarRole::Window );
addCommand( windowMenu, i18n( "previous_tab", "Previous Tab" ), "previous-tab" );
addCommand( windowMenu, i18n( "next_tab", "Next Tab" ), "next-tab" );
windowMenu->addSeparator();
addCommand( windowMenu, i18n( "split_left", "Split Left" ), "split-left" );
addCommand( windowMenu, i18n( "split_right", "Split Right" ), "split-right" );
addCommand( windowMenu, i18n( "split_top", "Split Top" ), "split-top" );
addCommand( windowMenu, i18n( "split_bottom", "Split Bottom" ), "split-bottom" );
windowMenu->on( Event::OnItemClicked, onItemClicked );
menuBar->addMenuButton( i18n( "window", "Window" ), windowMenu );
syncGlobalMenuKeybindings();
menuBar->setGlobalMenuBarEnabled( true );
}
#endif
bool App::closeWindow( EE::Window::Window* ) {
if ( closeApproved || !warnBeforeClose ) {
saveWindowState();
@@ -886,6 +994,9 @@ int App::run( int argc, char* argv[] ) {
return EXIT_FAILURE;
}
}
#if EE_PLATFORM == EE_PLATFORM_MACOS
createGlobalMenuBar();
#endif
if ( config->windowState.position != Vector2i( -1, -1 ) &&
config->windowState.displayIndex < displayManager->getDisplayCount() ) {
// 1 px offset to avoid a bug in SDL2 2.28 when maximizing windows

View File

@@ -121,6 +121,14 @@ class App : private efsw::FileWatchListener {
void addTabKeyBindings( UITerminal* terminal );
#if EE_PLATFORM == EE_PLATFORM_MACOS
void createGlobalMenuBar();
void executeMenuCommand( const std::string& command );
void syncGlobalMenuKeybindings();
#endif
KeyBindings::ShortcutMap getDefaultKeybindings() const;
void loadKeybindings();
@@ -149,6 +157,9 @@ class App : private efsw::FileWatchListener {
EE::Window::Window* appWindow{ nullptr };
UISceneNode* scene{ nullptr };
UILinearLayout* mainLayout{ nullptr };
#if EE_PLATFORM == EE_PLATFORM_MACOS
UIMenuBar* menuBar{ nullptr };
#endif
UITabWidgetSplitter* tabSplitter{ nullptr };
FontTrueType* terminalFont{ nullptr };
UIIcon* terminalIcon{ nullptr };

View File

@@ -132,6 +132,9 @@ void App::reloadKeybindings() {
forEachTerminal( [this]( UITerminal* terminal ) { applyKeybindings( terminal ); } );
if ( keybindingsEditor && tabSplitter->ownedWidgetExists( keybindingsEditor ) )
applyKeybindings( keybindingsEditor );
#if EE_PLATFORM == EE_PLATFORM_MACOS
syncGlobalMenuKeybindings();
#endif
}
void App::openKeybindings() {